[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM |
Date: |
Wed, 26 Feb 2020 04:53:33 -0500 |
On Wed, Feb 26, 2020 at 05:43:57PM +0800, Jason Wang wrote:
> We turn on device IOTLB via VIRTIO_F_IOMMU_PLATFORM unconditionally on
> platform without IOMMU support. This can lead unnecessary IOTLB
> transactions which will damage the performance.
>
> Fixing this by check whether the device is backed by IOMMU and disable
> device IOTLB.
>
> Reported-by: Halil Pasic <address@hidden>
> Fixes: c471ad0e9bd46 ("vhost_net: device IOTLB support")
Well it's just an optimization, isn't it?
I don't think it's justified to push this to everyone using
vhost with IOTLB, is it?
If you disagree, could you comment a bit on which configurations where tested?
> Cc: address@hidden
> Signed-off-by: Jason Wang <address@hidden>
Halil could you test this pls? Does this help your performance issue?
> ---
> Changes from V1:
> - do not check acked_features
> - reuse vhost_dev_has_iommu()
> ---
> hw/virtio/vhost.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 9edfadc81d..9182a00495 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -290,7 +290,14 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
> {
> VirtIODevice *vdev = dev->vdev;
>
> - return virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
> + /*
> + * For vhost, VIRTIO_F_IOMMU_PLATFORM means the backend support
> + * incremental memory mapping API via IOTLB API. For platform that
> + * does not have IOMMU, there's no need to enable this feature
> + * which may cause unnecessary IOTLB miss/update trnasactions.
> + */
> + return vdev->dma_as != &address_space_memory &&
> + virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
> }
>
> static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
> @@ -765,6 +772,9 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
> if (enable_log) {
> features |= 0x1ULL << VHOST_F_LOG_ALL;
> }
> + if (!vhost_dev_has_iommu(dev)) {
> + features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
> + }
> r = dev->vhost_ops->vhost_set_features(dev, features);
> if (r < 0) {
> VHOST_OPS_DEBUG("vhost_set_features failed");
> --
> 2.19.1