Re: [PATCH v4 1/1] virtio: fix the condition for iommu_platform not supported

[Halil Pasic]

On Wed,  2 Feb 2022 20:54:38 +0100
Halil Pasic <pasic@linux.ibm.com> wrote:

>      }
> @@ -82,9 +78,14 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
> **errp)
>          return;
>      }
>  
> +    vdev_has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
>      if (klass->get_dma_as != NULL && has_iommu) {
>          virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
>          vdev->dma_as = klass->get_dma_as(qbus->parent);
> +        if (!vdev_has_iommu && vdev->dma_as != &address_space_memory) {
> +            error_setg(errp,
> +                       "iommu_platform=true is not supported by the device");
> +        }

I'm wondering, would it be wise to change the message? Since this is now
dependent on the VM/bus the device is plugged into the message might be a
little misleading: i.e. the very same device could work perfectly fine
with iommu_platform=true if the "surroundings" are different.

Maybe "the device has no IOMMU support (iommu_platform=true)" would be a
better option. On the other hand changing the message has its downsides
as well.

Also I realized that keeping the return after error_setg() might have
been a good idea for the case more logic is added at the end of the
function.

In any case I would like to address these, if necessary with a separate
patch. I don't want the fix to experience any further delays.

Regards,
Halil