[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supp
From: |
Halil Pasic |
Subject: |
Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported |
Date: |
Wed, 2 Feb 2022 14:16:45 +0100 |
On Wed, 2 Feb 2022 02:06:12 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
[..]
> > In my opinion not forcing the guest to negotiate IOMMU_PLATFORM when
> > ->get_dma_as() is not set is at least unfortunate. Please observe, that
> > virtio-pci is not affected by this omission because for virtio-pci
> > devices ->get_dma_as != NULL always holds. And what is the deal for
> > devices that don't implement get_dma_as() (and don't need address
> > translation)? If iommu_platform=on is justified (no user error) then
> > the device does not have access to the entire guest memory. Which
> > means it more than likely needs cooperation form the guest (driver).
> > So detecting that the guest does not support IOMMU_PLATFORM and failing
> > gracefully via virtio_validate_features() instead of carrying on
> > in good faith and failing in ugly ways when the host attempts to access
> > guest memory to which it does not have access to. If we assume user
> > error, that is the host can access at least all the memory it needs
> > to access to make that device work, then it is probably still a
> > good idea to fail the device and thus help the user correct his
> > error.
> >
> > IMHO the best course of action is
> > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> > index 34f5a0a664..1d0eb16d1c 100644
> > --- a/hw/virtio/virtio-bus.c
> > +++ b/hw/virtio/virtio-bus.c
> > @@ -80,7 +80,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error
> > **errp)
> >
> > 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,
> > @@ -89,6 +88,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error
> > **errp)
> > } else {
> > vdev->dma_as = &address_space_memory;
> > }
> > + virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
> > }
> >
> > which would be a separate patch, as this is a separate issue. Jason,
> > Michael, Connie, what do you think?
>
> Do you mean just force VIRTIO_F_IOMMU_PLATFORM for everyone?
> Or am I misreading the patch?
Yes. Where force means: prevent the driver from setting FEATURES_OK
if it cleared VIRTIO_F_IOMMU_PLATFORM. I really don't see the case
where the device offering but the driver not accepting
VIRTIO_F_IOMMU_PLATFORM is good and useful.
Regards,
Halil
>
>
> > Regards,
> > Halil
>
>
- [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Halil Pasic, 2022/02/01
- Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Daniel Henrique Barboza, 2022/02/01
- Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Halil Pasic, 2022/02/01
- Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Daniel Henrique Barboza, 2022/02/02
- Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Halil Pasic, 2022/02/02
- Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Daniel Henrique Barboza, 2022/02/02
- Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Michael S. Tsirkin, 2022/02/02
Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Halil Pasic, 2022/02/01
Re: [PATCH v3 1/1] virtio: fix the condition for iommu_platform not supported, Cornelia Huck, 2022/02/01