Re: [Qemu-block] [PATCH for 2.8 02/11] virtio: convert to use DMA api

[Michael S. Tsirkin]

On Tue, Aug 30, 2016 at 09:31:27AM +0200, Cornelia Huck wrote:
> On Tue, 30 Aug 2016 11:06:50 +0800
> Jason Wang <address@hidden> 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 when iommu platform (VIRTIO_F_IOMMU_PLATFORM) was enabled
> >   for this device.
> > 
> > Cc: Michael S. Tsirkin <address@hidden>
> > Cc: Stefan Hajnoczi <address@hidden>
> > Cc: Kevin Wolf <address@hidden>
> > Cc: Amit Shah <address@hidden>
> > Cc: Paolo Bonzini <address@hidden>
> > Cc: address@hidden
> > Signed-off-by: Jason Wang <address@hidden>
> > ---
> >  hw/block/virtio-blk.c             |  2 +-
> >  hw/char/virtio-serial-bus.c       |  3 +-
> >  hw/scsi/virtio-scsi.c             |  4 ++-
> >  hw/virtio/virtio-pci.c            | 14 +++++++++
> >  hw/virtio/virtio.c                | 62 
> > ++++++++++++++++++++++++---------------
> >  include/hw/virtio/virtio-access.h | 43 ++++++++++++++++++++-------
> >  include/hw/virtio/virtio-bus.h    |  1 +
> >  include/hw/virtio/virtio.h        |  8 +++--
> >  8 files changed, 98 insertions(+), 39 deletions(-)
> > 
> 
> > diff --git a/include/hw/virtio/virtio-access.h 
> > b/include/hw/virtio/virtio-access.h
> > index 440b455..4071dad 100644
> > --- a/include/hw/virtio/virtio-access.h
> > +++ b/include/hw/virtio/virtio-access.h
> > @@ -17,12 +17,25 @@
> >  #define QEMU_VIRTIO_ACCESS_H
> > 
> >  #include "hw/virtio/virtio.h"
> > +#include "hw/virtio/virtio-bus.h"
> >  #include "exec/address-spaces.h"
> > 
> >  #if defined(TARGET_PPC64) || defined(TARGET_ARM)
> >  #define LEGACY_VIRTIO_IS_BIENDIAN 1
> >  #endif
> > 
> > +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 (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
> > +        k->get_dma_as) {
> > +        return k->get_dma_as(qbus->parent);
> > +    }
> > +    return &address_space_memory;
> > +}
> 
> One thing I'm a bit worried about is that we're introducing a check
> even if we know that the device will never support
> VIRTIO_F_IOMMU_PLATFORM (i.e. virtio-ccw). The qom incantations will
> add cycles to any invocation of this.

Yes - let's do container_of calls as opposed to QOM on data path.

> Is the address space likely to change during device lifetime? Can we
> cache it in some way?