[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] virtio: force VIRTIO_F_IOMMU_PLATFORM

From: Jason Wang
Subject: Re: [Qemu-devel] [PATCH] virtio: force VIRTIO_F_IOMMU_PLATFORM
Date: Thu, 19 Jan 2017 11:12:21 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1

On 2017年01月19日 01:50, Michael S. Tsirkin wrote:
On Wed, Jan 18, 2017 at 10:42:48AM +0800, Jason Wang wrote:

On 2017年01月17日 22:44, Michael S. Tsirkin wrote:
On Tue, Jan 17, 2017 at 12:01:00PM +0800, Jason Wang wrote:
We allow vhost to clear VIRITO_F_IOMMU_PLATFORM which is wrong since
VIRTIO_F_IOMMU_PLATFORM is mandatory for security. Fixing this by
enforce it after vdc->get_features().

Signed-off-by: Jason Wang <address@hidden>
   hw/virtio/virtio-bus.c | 5 +++--
   1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index d31cc00..a886011 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -47,6 +47,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
       VirtioBusState *bus = VIRTIO_BUS(qbus);
       VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
       VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+    bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
       DPRINTF("%s: plug device.\n", qbus->name);
@@ -63,8 +64,8 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
           klass->device_plugged(qbus->parent, errp);
-    if (klass->get_dma_as != NULL &&
-        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);
       } else {
           vdev->dma_as = &address_space_memory;
I suspect that's not enough, we must also fail or disable vhost
(depending on the options), otherwise things won't work.
Looks like with the patch, VIRTIO_F_IOMMU_PLATFORM will be passed to
Okay then. Could you please test on an old host kernel and confirm
what happens? Pls mention this in commit log.

Test were done before sending this patch :)

Vhost fallback to userspace since it lacks VIRTIO_F_IOMMU_PLATFORM.

So if vhost backend does not support it, it will fall
back to userspace (This may not work for vhost-user, but it's a bug existed
even before this patch).

I guess this is true but this suddenly makes it important to
fix this. So I prefer a patchset where patch 2 addresses
the fallback bug. In which way? Do you think we should
just fail if vhost doesn't work? Does this apply to both
userspace and kernel?

For 1.0 devices we could do this through NEEDS_RESET, but it doesn't work for legacy device. For kernel vhost, I'd prefer to keep the current fallback. For vhost-user, it seems not easy and we could not just terminate qemu in this case consider this kind of failure can only be detected during start not init.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]