On 11/23/2015 04:06 PM, Michael S. Tsirkin wrote: > On Mon, Nov 23, 2015 at 03:41:11PM +0800, Jason Wang wrote: >> Currently, all virtio devices bypass IOMMU completely. This is because >> address_space_memory is assumed and used during DMA emulation. This >> patch converts the virtio core API to use DMA API. This idea is >> >> - introducing a new transport specific helper to query the dma address >> space. (only pci version is implemented). >> - query and use this address space during virtio device guest memory >> accessing >> >> With this virtiodevices will not bypass IOMMU anymore. Little tested with >> intel_iommu=on with virtio guest DMA series posted in >> https://lkml.org/lkml/2015/10/28/64. >> >> TODO: >> - Feature bit for this > This probably implies it should only be implemented > if device supports the modern mode. > Not sure what's the best way to handle transitional > devices.
Don't see the issue here. Transitional devices could decide based on the feature bit? , for the name, how about something like VIRTIO_F_HOST_IOMMU ? > >> - Implement this for all transports > Tested with vhost, and it does not seem to work. > So more TODO: > - make this work with vhost-user and vhost-net. > > Also, it seems prudent to add > - make the new behaviour conditional on a new property Right. > >> Signed-off-by: Jason Wang <jasow...@redhat.com> >> --- >> hw/block/virtio-blk.c | 2 +- >> hw/char/virtio-serial-bus.c | 2 +- >> hw/scsi/virtio-scsi.c | 2 +- >> hw/virtio/virtio-pci.c | 9 +++++++++ >> hw/virtio/virtio.c | 36 +++++++++++++++++++-------------- >> include/hw/virtio/virtio-access.h | 42 >> +++++++++++++++++++++++++++++---------- >> include/hw/virtio/virtio-bus.h | 1 + >> include/hw/virtio/virtio.h | 2 +- >> 8 files changed, 67 insertions(+), 29 deletions(-) >> >> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c >> index e70fccf..5499480 100644 >> --- a/hw/block/virtio-blk.c >> +++ b/hw/block/virtio-blk.c >> @@ -846,7 +846,7 @@ static int virtio_blk_load_device(VirtIODevice *vdev, >> QEMUFile *f, >> req->next = s->rq; >> s->rq = req; >> >> - virtqueue_map(&req->elem); >> + virtqueue_map(vdev, &req->elem); >> } >> >> return 0; >> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c >> index 497b0af..39e9ed2 100644 >> --- a/hw/char/virtio-serial-bus.c >> +++ b/hw/char/virtio-serial-bus.c >> @@ -705,7 +705,7 @@ static int fetch_active_ports_list(QEMUFile *f, int >> version_id, >> >> qemu_get_buffer(f, (unsigned char *)&port->elem, >> sizeof(port->elem)); >> - virtqueue_map(&port->elem); >> + virtqueue_map(&s->parent_obj, &port->elem); >> >> /* >> * Port was throttled on source machine. Let's >> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c >> index 7655401..0734d27 100644 >> --- a/hw/scsi/virtio-scsi.c >> +++ b/hw/scsi/virtio-scsi.c >> @@ -206,7 +206,7 @@ static void *virtio_scsi_load_request(QEMUFile *f, >> SCSIRequest *sreq) >> req = virtio_scsi_init_req(s, vs->cmd_vqs[n]); >> qemu_get_buffer(f, (unsigned char *)&req->elem, sizeof(req->elem)); >> >> - virtqueue_map(&req->elem); >> + virtqueue_map(&vs->parent_obj, &req->elem); >> >> if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size, >> sizeof(VirtIOSCSICmdResp) + vs->sense_size) < >> 0) { >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index dd48562..876505d 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -1211,6 +1211,14 @@ static int virtio_pci_query_nvectors(DeviceState *d) >> return proxy->nvectors; >> } >> >> +static AddressSpace *virtio_pci_get_dma_as(DeviceState *d) >> +{ >> + VirtIOPCIProxy *proxy = VIRTIO_PCI(d); >> + PCIDevice *dev = &proxy->pci_dev; >> + >> + return pci_get_address_space(dev); >> +} >> + >> static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy, >> struct virtio_pci_cap *cap) >> { >> @@ -2476,6 +2484,7 @@ static void virtio_pci_bus_class_init(ObjectClass >> *klass, void *data) >> k->device_plugged = virtio_pci_device_plugged; >> k->device_unplugged = virtio_pci_device_unplugged; >> k->query_nvectors = virtio_pci_query_nvectors; >> + k->get_dma_as = virtio_pci_get_dma_as; >> } >> >> static const TypeInfo virtio_pci_bus_info = { >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index 1edef59..e09430d 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -21,6 +21,7 @@ >> #include "hw/virtio/virtio-bus.h" >> #include "migration/migration.h" >> #include "hw/virtio/virtio-access.h" >> +#include "sysemu/dma.h" >> >> /* >> * The alignment to use between consumer and producer parts of vring. >> @@ -247,6 +248,7 @@ int virtio_queue_empty(VirtQueue *vq) >> static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem, >> unsigned int len) >> { >> + AddressSpace *dma_as = virtio_get_dma_as(vq->vdev); >> unsigned int offset; >> int i; >> >> @@ -254,17 +256,17 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const >> VirtQueueElement *elem, >> for (i = 0; i < elem->in_num; i++) { >> size_t size = MIN(len - offset, elem->in_sg[i].iov_len); >> >> - cpu_physical_memory_unmap(elem->in_sg[i].iov_base, >> - elem->in_sg[i].iov_len, >> - 1, size); >> + dma_memory_unmap(dma_as, elem->in_sg[i].iov_base, >> elem->in_sg[i].iov_len, >> + DMA_DIRECTION_FROM_DEVICE, size); >> >> offset += size; >> } >> >> for (i = 0; i < elem->out_num; i++) >> - cpu_physical_memory_unmap(elem->out_sg[i].iov_base, >> - elem->out_sg[i].iov_len, >> - 0, elem->out_sg[i].iov_len); >> + dma_memory_unmap(dma_as, elem->out_sg[i].iov_base, >> + elem->out_sg[i].iov_len, >> + DMA_DIRECTION_TO_DEVICE, >> + elem->out_sg[i].iov_len); >> } >> >> void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem, >> @@ -448,9 +450,9 @@ int virtqueue_avail_bytes(VirtQueue *vq, unsigned int >> in_bytes, >> return in_bytes <= in_total && out_bytes <= out_total; >> } >> >> -static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr, >> - unsigned int *num_sg, unsigned int max_size, >> - int is_write) >> +static void virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg, >> + hwaddr *addr, unsigned int *num_sg, >> + unsigned int max_size, int is_write) >> { >> unsigned int i; >> hwaddr len; >> @@ -469,7 +471,10 @@ static void virtqueue_map_iovec(struct iovec *sg, >> hwaddr *addr, >> >> for (i = 0; i < *num_sg; i++) { >> len = sg[i].iov_len; >> - sg[i].iov_base = cpu_physical_memory_map(addr[i], &len, is_write); >> + sg[i].iov_base = dma_memory_map(virtio_get_dma_as(vdev), >> + addr[i], &len, is_write ? >> + DMA_DIRECTION_FROM_DEVICE : >> + DMA_DIRECTION_TO_DEVICE); >> if (!sg[i].iov_base) { >> error_report("virtio: error trying to map MMIO memory"); >> exit(1); >> @@ -491,13 +496,14 @@ static void virtqueue_map_iovec(struct iovec *sg, >> hwaddr *addr, >> } >> } >> >> -void virtqueue_map(VirtQueueElement *elem) >> +void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem) >> { >> - virtqueue_map_iovec(elem->in_sg, elem->in_addr, &elem->in_num, >> + virtqueue_map_iovec(vdev, elem->in_sg, elem->in_addr, &elem->in_num, >> MIN(ARRAY_SIZE(elem->in_sg), >> ARRAY_SIZE(elem->in_addr)), >> 1); >> - virtqueue_map_iovec(elem->out_sg, elem->out_addr, &elem->out_num, >> - MIN(ARRAY_SIZE(elem->out_sg), >> ARRAY_SIZE(elem->out_addr)), >> + virtqueue_map_iovec(vdev, elem->out_sg, elem->out_addr, &elem->out_num, >> + MIN(ARRAY_SIZE(elem->out_sg), >> + ARRAY_SIZE(elem->out_addr)), >> 0); >> } >> >> @@ -562,7 +568,7 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem) >> } while ((i = virtqueue_next_desc(vdev, desc_pa, i, max)) != max); >> >> /* Now map what we have collected */ >> - virtqueue_map(elem); >> + virtqueue_map(vdev, elem); >> >> elem->index = head; >> >> diff --git a/include/hw/virtio/virtio-access.h >> b/include/hw/virtio/virtio-access.h >> index 8aec843..cca7d2a 100644 >> --- a/include/hw/virtio/virtio-access.h >> +++ b/include/hw/virtio/virtio-access.h >> @@ -15,8 +15,20 @@ >> #ifndef _QEMU_VIRTIO_ACCESS_H >> #define _QEMU_VIRTIO_ACCESS_H >> #include "hw/virtio/virtio.h" >> +#include "hw/virtio/virtio-bus.h" >> #include "exec/address-spaces.h" >> >> +static inline AddressSpace *virtio_get_dma_as(VirtIODevice *vdev) >> +{ >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >> + >> + if (k->get_dma_as) { >> + return k->get_dma_as(qbus->parent); >> + } >> + return &address_space_memory; >> +} >> + >> static inline bool virtio_access_is_big_endian(VirtIODevice *vdev) >> { >> if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { >> @@ -47,45 +59,55 @@ static inline bool >> virtio_legacy_is_cross_endian(VirtIODevice *vdev) >> >> static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa) >> { >> + AddressSpace *dma_as = virtio_get_dma_as(vdev); >> + >> if (virtio_access_is_big_endian(vdev)) { >> - return lduw_be_phys(&address_space_memory, pa); >> + return lduw_be_phys(dma_as, pa); >> } >> - return lduw_le_phys(&address_space_memory, pa); >> + return lduw_le_phys(dma_as, pa); >> } >> >> static inline uint32_t virtio_ldl_phys(VirtIODevice *vdev, hwaddr pa) >> { >> + AddressSpace *dma_as = virtio_get_dma_as(vdev); >> + >> if (virtio_access_is_big_endian(vdev)) { >> - return ldl_be_phys(&address_space_memory, pa); >> + return ldl_be_phys(dma_as, pa); >> } >> - return ldl_le_phys(&address_space_memory, pa); >> + return ldl_le_phys(dma_as, pa); >> } >> >> static inline uint64_t virtio_ldq_phys(VirtIODevice *vdev, hwaddr pa) >> { >> + AddressSpace *dma_as = virtio_get_dma_as(vdev); >> + >> if (virtio_access_is_big_endian(vdev)) { >> - return ldq_be_phys(&address_space_memory, pa); >> + return ldq_be_phys(dma_as, pa); >> } >> - return ldq_le_phys(&address_space_memory, pa); >> + return ldq_le_phys(dma_as, pa); >> } >> >> static inline void virtio_stw_phys(VirtIODevice *vdev, hwaddr pa, >> uint16_t value) >> { >> + AddressSpace *dma_as = virtio_get_dma_as(vdev); >> + >> if (virtio_access_is_big_endian(vdev)) { >> - stw_be_phys(&address_space_memory, pa, value); >> + stw_be_phys(dma_as, pa, value); >> } else { >> - stw_le_phys(&address_space_memory, pa, value); >> + stw_le_phys(dma_as, pa, value); >> } >> } >> >> static inline void virtio_stl_phys(VirtIODevice *vdev, hwaddr pa, >> uint32_t value) >> { >> + AddressSpace *dma_as = virtio_get_dma_as(vdev); >> + >> if (virtio_access_is_big_endian(vdev)) { >> - stl_be_phys(&address_space_memory, pa, value); >> + stl_be_phys(dma_as, pa, value); >> } else { >> - stl_le_phys(&address_space_memory, pa, value); >> + stl_le_phys(dma_as, pa, value); >> } >> } >> >> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h >> index 6c3d4cb..638735e 100644 >> --- a/include/hw/virtio/virtio-bus.h >> +++ b/include/hw/virtio/virtio-bus.h >> @@ -71,6 +71,7 @@ typedef struct VirtioBusClass { >> * Note that changing this will break migration for this transport. >> */ >> bool has_variable_vring_alignment; >> + AddressSpace *(*get_dma_as)(DeviceState *d); >> } VirtioBusClass; >> >> struct VirtioBusState { >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h >> index 205fadf..7a8ef13 100644 >> --- a/include/hw/virtio/virtio.h >> +++ b/include/hw/virtio/virtio.h >> @@ -151,7 +151,7 @@ void virtqueue_discard(VirtQueue *vq, const >> VirtQueueElement *elem, >> void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem, >> unsigned int len, unsigned int idx); >> >> -void virtqueue_map(VirtQueueElement *elem); >> +void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem); >> int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem); >> int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes, >> unsigned int out_bytes); >> -- >> 2.5.0