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

2017-01-20 Thread Michael S. Tsirkin 
On Fri, Jan 20, 2017 at 11:28:53AM +0800, Jason Wang wrote:
> 
> 
> On 2017年01月20日 06:27, Michael S. Tsirkin wrote:
> > On Thu, Jan 19, 2017 at 11:12:21AM +0800, Jason Wang wrote:
> > > 
> > > 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 
> > > > > > > ---
> > > > > > > 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 **errp)
> > > > > > > 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 **errp)
> > > > > > > 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(>host_features, 
> > > > > > > VIRTIO_F_IOMMU_PLATFORM);
> > > > > > > vdev->dma_as = klass->get_dma_as(qbus->parent);
> > > > > > > } else {
> > > > > > > vdev->dma_as = _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
> > > > > vhost_set_features().
> > > > 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).
> > > > > 
> > > > > Thanks
> > > > 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.
> > I'm not so sure. This has security implications, e.g. user might
> > expect not to be affected by a bug in qemu if specifying vhost=on.
> 
> Ok.
> 
> > 
> > > 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.
> > > 
> > > Thanks
> > I think we should at least deny connection.
> 
> Ok and we need do something after denying it, how about vdev->broken or
> others?
> 
> Thanks

For vhost-user qemu does not make progress until there's
a valid connection.

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

2017-01-19 Thread Jason Wang 



On 2017年01月20日 06:27, Michael S. Tsirkin wrote:

On Thu, Jan 19, 2017 at 11:12:21AM +0800, Jason Wang wrote:


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 
---
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 
**errp)
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 
**errp)
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(>host_features, VIRTIO_F_IOMMU_PLATFORM);
vdev->dma_as = klass->get_dma_as(qbus->parent);
} else {
vdev->dma_as = _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
vhost_set_features().

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).

Thanks

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.

I'm not so sure. This has security implications, e.g. user might
expect not to be affected by a bug in qemu if specifying vhost=on.


Ok.




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.

Thanks

I think we should at least deny connection.


Ok and we need do something after denying it, how about vdev->broken or 
others?


Thanks

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

2017-01-19 Thread Michael S. Tsirkin 
On Thu, Jan 19, 2017 at 11:12:21AM +0800, Jason Wang wrote:
> 
> 
> 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 
> > > > > ---
> > > > >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 **errp)
> > > > >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 **errp)
> > > > >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(>host_features, 
> > > > > VIRTIO_F_IOMMU_PLATFORM);
> > > > >vdev->dma_as = klass->get_dma_as(qbus->parent);
> > > > >} else {
> > > > >vdev->dma_as = _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
> > > vhost_set_features().
> > 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).
> > > 
> > > Thanks
> > 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.

I'm not so sure. This has security implications, e.g. user might
expect not to be affected by a bug in qemu if specifying vhost=on.

> 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.
> 
> Thanks

I think we should at least deny connection.

-- 
MST

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

2017-01-18 Thread Jason Wang 



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 
---
   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 
**errp)
   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 
**errp)
   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(>host_features, VIRTIO_F_IOMMU_PLATFORM);
   vdev->dma_as = klass->get_dma_as(qbus->parent);
   } else {
   vdev->dma_as = _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
vhost_set_features().

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).

Thanks

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.


Thanks

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

2017-01-18 Thread Michael S. Tsirkin 
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 
> > > ---
> > >   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 **errp)
> > >   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 **errp)
> > >   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(>host_features, 
> > > VIRTIO_F_IOMMU_PLATFORM);
> > >   vdev->dma_as = klass->get_dma_as(qbus->parent);
> > >   } else {
> > >   vdev->dma_as = _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
> vhost_set_features().

Okay then. Could you please test on an old host kernel and confirm
what happens? Pls mention this in commit log.

> 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).
> 
> Thanks

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?

-- 
MST

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

2017-01-17 Thread Jason Wang 



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 
---
  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 
**errp)
  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 **errp)

  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(>host_features, VIRTIO_F_IOMMU_PLATFORM);
  vdev->dma_as = klass->get_dma_as(qbus->parent);
  } else {
  vdev->dma_as = _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 
vhost_set_features(). 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).


Thanks

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

2017-01-17 Thread Michael S. Tsirkin 
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 
> ---
>  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 
> **errp)
>  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 
> **errp)
>  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(>host_features, VIRTIO_F_IOMMU_PLATFORM);
>  vdev->dma_as = klass->get_dma_as(qbus->parent);
>  } else {
>  vdev->dma_as = _space_memory;

I suspect that's not enough, we must also fail or disable vhost
(depending on the options), otherwise things won't work.

> -- 
> 2.7.4

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

2017-01-16 Thread Jason Wang 
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 
---
 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 
**errp)
 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 
**errp)
 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(>host_features, VIRTIO_F_IOMMU_PLATFORM);
 vdev->dma_as = klass->get_dma_as(qbus->parent);
 } else {
 vdev->dma_as = _space_memory;
-- 
2.7.4