On Wed, 2 Feb 2022 20:54:38 +0100 Halil Pasic <pa...@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