On Thu, Feb 03, 2022 at 05:06:35PM +0100, Halil Pasic wrote:
> 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.
I personally don't care much frankly.
> 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.
OK so you are sending v5 with this change then?
> 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
