| Message ID | 20220207112857.607829-1-pasic@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,1/1] virtio: fix the condition for iommu_platform not supported | expand |
On Mon, Feb 7, 2022 at 7:29 PM Halil Pasic <pasic@linux.ibm.com> wrote: > > The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > unsupported") claims to fail the device hotplug when iommu_platform > is requested, but not supported by the (vhost) device. On the first > glance the condition for detecting that situation looks perfect, but > because a certain peculiarity of virtio_platform it ain't. > > In fact the aforementioned commit introduces a regression. It breaks > virtio-fs support for Secure Execution, and most likely also for AMD SEV > or any other confidential guest scenario that relies encrypted guest > memory. The same also applies to any other vhost device that does not > support _F_ACCESS_PLATFORM. > > The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates > "device can not access all of the guest RAM" and "iova != gpa, thus > device needs to translate iova". > > Confidential guest technologies currently rely on the device/hypervisor > offering _F_ACCESS_PLATFORM, so that, after the feature has been > negotiated, the guest grants access to the portions of memory the > device needs to see. So in for confidential guests, generally, > _F_ACCESS_PLATFORM is about the restricted access to memory, but not > about the addresses used being something else than guest physical > addresses. > > This is the very reason for which commit f7ef7e6e3b ("vhost: correctly > turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the > vhost device that does not need it, because on the vhost interface it > only means "I/O address translation is needed". > > This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on > VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the > situation when _F_ACCESS_PLATFORM is requested, but no I/O translation > by the device, and thus no device capability is needed. In this > situation claiming that the device does not support iommu_plattform=on > is counter-productive. So let us stop doing that! > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com> > Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > unsupported") > Acked-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: qemu-stable@nongnu.org Acked-by: Jason Wang <jasowang@redhat.com> > > --- > > v4->v5: > * added back the return; so if somebody were to add code to the end of > the function we are still good > v3->v4: > * Fixed commit message (thanks Connie) > * Removed counter-productive initialization (thanks Connie) > * Added tags > v2->v3: > * Caught a bug: I tired to check if vdev has the feature > ACCESS_PLATFORM after we have forced it. Moved the check > to a better place > v1->v2: > * Commit message tweaks. Most notably fixed commit SHA (Michael) > > --- > --- > hw/virtio/virtio-bus.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > index d23db98c56..0f69d1c742 100644 > --- a/hw/virtio/virtio-bus.c > +++ b/hw/virtio/virtio-bus.c > @@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) > 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); > + bool vdev_has_iommu; > Error *local_err = NULL; > > DPRINTF("%s: plug device.\n", qbus->name); > @@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) > return; > } > > - if (has_iommu && !virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { > - error_setg(errp, "iommu_platform=true is not supported by the device"); > - return; > - } > - > if (klass->device_plugged != NULL) { > klass->device_plugged(qbus->parent, &local_err); > } > @@ -82,9 +78,15 @@ 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"); > + return; > + } > } else { > vdev->dma_as = &address_space_memory; > } > > base-commit: 0d564a3e32ba8494014c67cdd2ebf0fb71860dff > -- > 2.32.0 >
On 2/7/2022 7:28 PM, Halil Pasic wrote: > The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > unsupported") claims to fail the device hotplug when iommu_platform > is requested, but not supported by the (vhost) device. On the first > glance the condition for detecting that situation looks perfect, but > because a certain peculiarity of virtio_platform it ain't. > > In fact the aforementioned commit introduces a regression. It breaks > virtio-fs support for Secure Execution, and most likely also for AMD SEV > or any other confidential guest scenario that relies encrypted guest > memory. The same also applies to any other vhost device that does not > support _F_ACCESS_PLATFORM. > > The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates > "device can not access all of the guest RAM" and "iova != gpa, thus > device needs to translate iova". > > Confidential guest technologies currently rely on the device/hypervisor > offering _F_ACCESS_PLATFORM, so that, after the feature has been > negotiated, the guest grants access to the portions of memory the > device needs to see. So in for confidential guests, generally, > _F_ACCESS_PLATFORM is about the restricted access to memory, but not > about the addresses used being something else than guest physical > addresses. > > This is the very reason for which commit f7ef7e6e3b ("vhost: correctly > turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the > vhost device that does not need it, because on the vhost interface it > only means "I/O address translation is needed". > > This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on > VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the > situation when _F_ACCESS_PLATFORM is requested, but no I/O translation > by the device, and thus no device capability is needed. In this > situation claiming that the device does not support iommu_plattform=on > is counter-productive. So let us stop doing that! > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com> > Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > unsupported") > Acked-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: qemu-stable@nongnu.org > > --- > > v4->v5: > * added back the return; so if somebody were to add code to the end of > the function we are still good > v3->v4: > * Fixed commit message (thanks Connie) > * Removed counter-productive initialization (thanks Connie) > * Added tags > v2->v3: > * Caught a bug: I tired to check if vdev has the feature > ACCESS_PLATFORM after we have forced it. Moved the check > to a better place > v1->v2: > * Commit message tweaks. Most notably fixed commit SHA (Michael) > > --- > --- > hw/virtio/virtio-bus.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > index d23db98c56..0f69d1c742 100644 > --- a/hw/virtio/virtio-bus.c > +++ b/hw/virtio/virtio-bus.c > @@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) > 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); > + bool vdev_has_iommu; > Error *local_err = NULL; > > DPRINTF("%s: plug device.\n", qbus->name); > @@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) > return; > } > > - if (has_iommu && !virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { > - error_setg(errp, "iommu_platform=true is not supported by the device"); > - return; > - } > - > if (klass->device_plugged != NULL) { > klass->device_plugged(qbus->parent, &local_err); > } > @@ -82,9 +78,15 @@ 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) { Hi Pasic, When testing the virtio-fs in Intel TDX, I met the error report in this check. Is it appropriate to compare the dma_as against the address_space_memory to detect whether the IOMMU is enabled or not? Per the commit ae4003738f(vhost: correctly detect the enabling IOMMU), we should call virtio_bus_device_iommu_enabled(vdev) instead here, correct? > + error_setg(errp, > + "iommu_platform=true is not supported by the device"); > + return; > + } > } else { > vdev->dma_as = &address_space_memory; > } > > base-commit: 0d564a3e32ba8494014c67cdd2ebf0fb71860dff
On 4/22/2022 3:11 PM, Chenyi Qiang wrote: > > > On 2/7/2022 7:28 PM, Halil Pasic wrote: >> The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but >> unsupported") claims to fail the device hotplug when iommu_platform >> is requested, but not supported by the (vhost) device. On the first >> glance the condition for detecting that situation looks perfect, but >> because a certain peculiarity of virtio_platform it ain't. >> >> In fact the aforementioned commit introduces a regression. It breaks >> virtio-fs support for Secure Execution, and most likely also for AMD SEV >> or any other confidential guest scenario that relies encrypted guest >> memory. The same also applies to any other vhost device that does not >> support _F_ACCESS_PLATFORM. >> >> The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates >> "device can not access all of the guest RAM" and "iova != gpa, thus >> device needs to translate iova". >> >> Confidential guest technologies currently rely on the device/hypervisor >> offering _F_ACCESS_PLATFORM, so that, after the feature has been >> negotiated, the guest grants access to the portions of memory the >> device needs to see. So in for confidential guests, generally, >> _F_ACCESS_PLATFORM is about the restricted access to memory, but not >> about the addresses used being something else than guest physical >> addresses. >> >> This is the very reason for which commit f7ef7e6e3b ("vhost: correctly >> turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the >> vhost device that does not need it, because on the vhost interface it >> only means "I/O address translation is needed". >> >> This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on >> VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the >> situation when _F_ACCESS_PLATFORM is requested, but no I/O translation >> by the device, and thus no device capability is needed. In this >> situation claiming that the device does not support iommu_plattform=on >> is counter-productive. So let us stop doing that! >> >> Signed-off-by: Halil Pasic <pasic@linux.ibm.com> >> Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com> >> Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but >> unsupported") >> Acked-by: Cornelia Huck <cohuck@redhat.com> >> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> Cc: Kevin Wolf <kwolf@redhat.com> >> Cc: qemu-stable@nongnu.org >> >> --- >> >> v4->v5: >> * added back the return; so if somebody were to add code to the end of >> the function we are still good >> v3->v4: >> * Fixed commit message (thanks Connie) >> * Removed counter-productive initialization (thanks Connie) >> * Added tags >> v2->v3: >> * Caught a bug: I tired to check if vdev has the feature >> ACCESS_PLATFORM after we have forced it. Moved the check >> to a better place >> v1->v2: >> * Commit message tweaks. Most notably fixed commit SHA (Michael) >> >> --- >> --- >> hw/virtio/virtio-bus.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c >> index d23db98c56..0f69d1c742 100644 >> --- a/hw/virtio/virtio-bus.c >> +++ b/hw/virtio/virtio-bus.c >> @@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, >> Error **errp) >> 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); >> + bool vdev_has_iommu; >> Error *local_err = NULL; >> DPRINTF("%s: plug device.\n", qbus->name); >> @@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, >> Error **errp) >> return; >> } >> - if (has_iommu && !virtio_host_has_feature(vdev, >> VIRTIO_F_IOMMU_PLATFORM)) { >> - error_setg(errp, "iommu_platform=true is not supported by the >> device"); >> - return; >> - } >> - >> if (klass->device_plugged != NULL) { >> klass->device_plugged(qbus->parent, &local_err); >> } >> @@ -82,9 +78,15 @@ 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) { > > Hi Pasic, > > When testing the virtio-fs in Intel TDX, I met the error report in this > check. Is it appropriate to compare the dma_as against the > address_space_memory to detect whether the IOMMU is enabled or not? Per > the commit ae4003738f(vhost: correctly detect the enabling IOMMU), we > should call virtio_bus_device_iommu_enabled(vdev) instead here, correct? > Sorry for bothering. Can virtio-fs work properly in AMD SEV? IIUC, If get_dma_as() is implemented and in case of PCI, pci_get_address_space() is used and returns the bus master as. This would fail the check here. >> + error_setg(errp, >> + "iommu_platform=true is not supported by the >> device"); >> + return; >> + } >> } else { >> vdev->dma_as = &address_space_memory; >> } >> >> base-commit: 0d564a3e32ba8494014c67cdd2ebf0fb71860dff >
On Wed, Apr 27, 2022 at 8:25 PM Chenyi Qiang <chenyi.qiang@intel.com> wrote: > > > > On 4/22/2022 3:11 PM, Chenyi Qiang wrote: > > > > > > On 2/7/2022 7:28 PM, Halil Pasic wrote: > >> The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > >> unsupported") claims to fail the device hotplug when iommu_platform > >> is requested, but not supported by the (vhost) device. On the first > >> glance the condition for detecting that situation looks perfect, but > >> because a certain peculiarity of virtio_platform it ain't. > >> > >> In fact the aforementioned commit introduces a regression. It breaks > >> virtio-fs support for Secure Execution, and most likely also for AMD SEV > >> or any other confidential guest scenario that relies encrypted guest > >> memory. The same also applies to any other vhost device that does not > >> support _F_ACCESS_PLATFORM. > >> > >> The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates > >> "device can not access all of the guest RAM" and "iova != gpa, thus > >> device needs to translate iova". > >> > >> Confidential guest technologies currently rely on the device/hypervisor > >> offering _F_ACCESS_PLATFORM, so that, after the feature has been > >> negotiated, the guest grants access to the portions of memory the > >> device needs to see. So in for confidential guests, generally, > >> _F_ACCESS_PLATFORM is about the restricted access to memory, but not > >> about the addresses used being something else than guest physical > >> addresses. > >> > >> This is the very reason for which commit f7ef7e6e3b ("vhost: correctly > >> turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the > >> vhost device that does not need it, because on the vhost interface it > >> only means "I/O address translation is needed". > >> > >> This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on > >> VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the > >> situation when _F_ACCESS_PLATFORM is requested, but no I/O translation > >> by the device, and thus no device capability is needed. In this > >> situation claiming that the device does not support iommu_plattform=on > >> is counter-productive. So let us stop doing that! > >> > >> Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > >> Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com> > >> Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > >> unsupported") > >> Acked-by: Cornelia Huck <cohuck@redhat.com> > >> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > >> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> > >> Cc: Kevin Wolf <kwolf@redhat.com> > >> Cc: qemu-stable@nongnu.org > >> > >> --- > >> > >> v4->v5: > >> * added back the return; so if somebody were to add code to the end of > >> the function we are still good > >> v3->v4: > >> * Fixed commit message (thanks Connie) > >> * Removed counter-productive initialization (thanks Connie) > >> * Added tags > >> v2->v3: > >> * Caught a bug: I tired to check if vdev has the feature > >> ACCESS_PLATFORM after we have forced it. Moved the check > >> to a better place > >> v1->v2: > >> * Commit message tweaks. Most notably fixed commit SHA (Michael) > >> > >> --- > >> --- > >> hw/virtio/virtio-bus.c | 12 +++++++----- > >> 1 file changed, 7 insertions(+), 5 deletions(-) > >> > >> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > >> index d23db98c56..0f69d1c742 100644 > >> --- a/hw/virtio/virtio-bus.c > >> +++ b/hw/virtio/virtio-bus.c > >> @@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, > >> Error **errp) > >> 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); > >> + bool vdev_has_iommu; > >> Error *local_err = NULL; > >> DPRINTF("%s: plug device.\n", qbus->name); > >> @@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, > >> Error **errp) > >> return; > >> } > >> - if (has_iommu && !virtio_host_has_feature(vdev, > >> VIRTIO_F_IOMMU_PLATFORM)) { > >> - error_setg(errp, "iommu_platform=true is not supported by the > >> device"); > >> - return; > >> - } > >> - > >> if (klass->device_plugged != NULL) { > >> klass->device_plugged(qbus->parent, &local_err); > >> } > >> @@ -82,9 +78,15 @@ 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) { > > > > Hi Pasic, > > > > When testing the virtio-fs in Intel TDX, I met the error report in this > > check. Is it appropriate to compare the dma_as against the > > address_space_memory to detect whether the IOMMU is enabled or not? Per > > the commit ae4003738f(vhost: correctly detect the enabling IOMMU), we > > should call virtio_bus_device_iommu_enabled(vdev) instead here, correct? > > > > Sorry for bothering. > > Can virtio-fs work properly in AMD SEV? > > IIUC, If get_dma_as() is implemented and in case of PCI, > pci_get_address_space() is used and returns the bus master as. This > would fail the check here. I think the reason is that the viritio-fs is used without vIOMMU but ACCESS_PLATFORM. That's why we need to use virtio_bus_device_iommu_enabled() to allow this setup to work. Thanks > > >> + error_setg(errp, > >> + "iommu_platform=true is not supported by the > >> device"); > >> + return; > >> + } > >> } else { > >> vdev->dma_as = &address_space_memory; > >> } > >> > >> base-commit: 0d564a3e32ba8494014c67cdd2ebf0fb71860dff > > >
On Thu, Apr 28, 2022 at 11:01:10AM +0800, Jason Wang wrote: > On Wed, Apr 27, 2022 at 8:25 PM Chenyi Qiang <chenyi.qiang@intel.com> wrote: > > > > > > > > On 4/22/2022 3:11 PM, Chenyi Qiang wrote: > > > > > > > > > On 2/7/2022 7:28 PM, Halil Pasic wrote: > > >> The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > > >> unsupported") claims to fail the device hotplug when iommu_platform > > >> is requested, but not supported by the (vhost) device. On the first > > >> glance the condition for detecting that situation looks perfect, but > > >> because a certain peculiarity of virtio_platform it ain't. > > >> > > >> In fact the aforementioned commit introduces a regression. It breaks > > >> virtio-fs support for Secure Execution, and most likely also for AMD SEV > > >> or any other confidential guest scenario that relies encrypted guest > > >> memory. The same also applies to any other vhost device that does not > > >> support _F_ACCESS_PLATFORM. > > >> > > >> The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates > > >> "device can not access all of the guest RAM" and "iova != gpa, thus > > >> device needs to translate iova". > > >> > > >> Confidential guest technologies currently rely on the device/hypervisor > > >> offering _F_ACCESS_PLATFORM, so that, after the feature has been > > >> negotiated, the guest grants access to the portions of memory the > > >> device needs to see. So in for confidential guests, generally, > > >> _F_ACCESS_PLATFORM is about the restricted access to memory, but not > > >> about the addresses used being something else than guest physical > > >> addresses. > > >> > > >> This is the very reason for which commit f7ef7e6e3b ("vhost: correctly > > >> turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the > > >> vhost device that does not need it, because on the vhost interface it > > >> only means "I/O address translation is needed". > > >> > > >> This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on > > >> VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the > > >> situation when _F_ACCESS_PLATFORM is requested, but no I/O translation > > >> by the device, and thus no device capability is needed. In this > > >> situation claiming that the device does not support iommu_plattform=on > > >> is counter-productive. So let us stop doing that! > > >> > > >> Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > > >> Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com> > > >> Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > > >> unsupported") > > >> Acked-by: Cornelia Huck <cohuck@redhat.com> > > >> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > >> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > >> Cc: Kevin Wolf <kwolf@redhat.com> > > >> Cc: qemu-stable@nongnu.org > > >> > > >> --- > > >> > > >> v4->v5: > > >> * added back the return; so if somebody were to add code to the end of > > >> the function we are still good > > >> v3->v4: > > >> * Fixed commit message (thanks Connie) > > >> * Removed counter-productive initialization (thanks Connie) > > >> * Added tags > > >> v2->v3: > > >> * Caught a bug: I tired to check if vdev has the feature > > >> ACCESS_PLATFORM after we have forced it. Moved the check > > >> to a better place > > >> v1->v2: > > >> * Commit message tweaks. Most notably fixed commit SHA (Michael) > > >> > > >> --- > > >> --- > > >> hw/virtio/virtio-bus.c | 12 +++++++----- > > >> 1 file changed, 7 insertions(+), 5 deletions(-) > > >> > > >> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > > >> index d23db98c56..0f69d1c742 100644 > > >> --- a/hw/virtio/virtio-bus.c > > >> +++ b/hw/virtio/virtio-bus.c > > >> @@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, > > >> Error **errp) > > >> 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); > > >> + bool vdev_has_iommu; > > >> Error *local_err = NULL; > > >> DPRINTF("%s: plug device.\n", qbus->name); > > >> @@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, > > >> Error **errp) > > >> return; > > >> } > > >> - if (has_iommu && !virtio_host_has_feature(vdev, > > >> VIRTIO_F_IOMMU_PLATFORM)) { > > >> - error_setg(errp, "iommu_platform=true is not supported by the > > >> device"); > > >> - return; > > >> - } > > >> - > > >> if (klass->device_plugged != NULL) { > > >> klass->device_plugged(qbus->parent, &local_err); > > >> } > > >> @@ -82,9 +78,15 @@ 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) { > > > > > > Hi Pasic, > > > > > > When testing the virtio-fs in Intel TDX, I met the error report in this > > > check. Is it appropriate to compare the dma_as against the > > > address_space_memory to detect whether the IOMMU is enabled or not? Per > > > the commit ae4003738f(vhost: correctly detect the enabling IOMMU), we > > > should call virtio_bus_device_iommu_enabled(vdev) instead here, correct? > > > > > > > Sorry for bothering. > > > > Can virtio-fs work properly in AMD SEV? > > > > IIUC, If get_dma_as() is implemented and in case of PCI, > > pci_get_address_space() is used and returns the bus master as. This > > would fail the check here. > > I think the reason is that the viritio-fs is used without vIOMMU but > ACCESS_PLATFORM. > > That's why we need to use virtio_bus_device_iommu_enabled() to allow > this setup to work. > > Thanks Do you retract your ack then? > > > > >> + error_setg(errp, > > >> + "iommu_platform=true is not supported by the > > >> device"); > > >> + return; > > >> + } > > >> } else { > > >> vdev->dma_as = &address_space_memory; > > >> } > > >> > > >> base-commit: 0d564a3e32ba8494014c67cdd2ebf0fb71860dff > > > > >
On Thu, Apr 28, 2022 at 12:57 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Thu, Apr 28, 2022 at 11:01:10AM +0800, Jason Wang wrote: > > On Wed, Apr 27, 2022 at 8:25 PM Chenyi Qiang <chenyi.qiang@intel.com> wrote: > > > > > > > > > > > > On 4/22/2022 3:11 PM, Chenyi Qiang wrote: > > > > > > > > > > > > On 2/7/2022 7:28 PM, Halil Pasic wrote: > > > >> The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > > > >> unsupported") claims to fail the device hotplug when iommu_platform > > > >> is requested, but not supported by the (vhost) device. On the first > > > >> glance the condition for detecting that situation looks perfect, but > > > >> because a certain peculiarity of virtio_platform it ain't. > > > >> > > > >> In fact the aforementioned commit introduces a regression. It breaks > > > >> virtio-fs support for Secure Execution, and most likely also for AMD SEV > > > >> or any other confidential guest scenario that relies encrypted guest > > > >> memory. The same also applies to any other vhost device that does not > > > >> support _F_ACCESS_PLATFORM. > > > >> > > > >> The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates > > > >> "device can not access all of the guest RAM" and "iova != gpa, thus > > > >> device needs to translate iova". > > > >> > > > >> Confidential guest technologies currently rely on the device/hypervisor > > > >> offering _F_ACCESS_PLATFORM, so that, after the feature has been > > > >> negotiated, the guest grants access to the portions of memory the > > > >> device needs to see. So in for confidential guests, generally, > > > >> _F_ACCESS_PLATFORM is about the restricted access to memory, but not > > > >> about the addresses used being something else than guest physical > > > >> addresses. > > > >> > > > >> This is the very reason for which commit f7ef7e6e3b ("vhost: correctly > > > >> turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the > > > >> vhost device that does not need it, because on the vhost interface it > > > >> only means "I/O address translation is needed". > > > >> > > > >> This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on > > > >> VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the > > > >> situation when _F_ACCESS_PLATFORM is requested, but no I/O translation > > > >> by the device, and thus no device capability is needed. In this > > > >> situation claiming that the device does not support iommu_plattform=on > > > >> is counter-productive. So let us stop doing that! > > > >> > > > >> Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > > > >> Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com> > > > >> Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > > > >> unsupported") > > > >> Acked-by: Cornelia Huck <cohuck@redhat.com> > > > >> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > > >> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > > >> Cc: Kevin Wolf <kwolf@redhat.com> > > > >> Cc: qemu-stable@nongnu.org > > > >> > > > >> --- > > > >> > > > >> v4->v5: > > > >> * added back the return; so if somebody were to add code to the end of > > > >> the function we are still good > > > >> v3->v4: > > > >> * Fixed commit message (thanks Connie) > > > >> * Removed counter-productive initialization (thanks Connie) > > > >> * Added tags > > > >> v2->v3: > > > >> * Caught a bug: I tired to check if vdev has the feature > > > >> ACCESS_PLATFORM after we have forced it. Moved the check > > > >> to a better place > > > >> v1->v2: > > > >> * Commit message tweaks. Most notably fixed commit SHA (Michael) > > > >> > > > >> --- > > > >> --- > > > >> hw/virtio/virtio-bus.c | 12 +++++++----- > > > >> 1 file changed, 7 insertions(+), 5 deletions(-) > > > >> > > > >> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > > > >> index d23db98c56..0f69d1c742 100644 > > > >> --- a/hw/virtio/virtio-bus.c > > > >> +++ b/hw/virtio/virtio-bus.c > > > >> @@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, > > > >> Error **errp) > > > >> 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); > > > >> + bool vdev_has_iommu; > > > >> Error *local_err = NULL; > > > >> DPRINTF("%s: plug device.\n", qbus->name); > > > >> @@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, > > > >> Error **errp) > > > >> return; > > > >> } > > > >> - if (has_iommu && !virtio_host_has_feature(vdev, > > > >> VIRTIO_F_IOMMU_PLATFORM)) { > > > >> - error_setg(errp, "iommu_platform=true is not supported by the > > > >> device"); > > > >> - return; > > > >> - } > > > >> - > > > >> if (klass->device_plugged != NULL) { > > > >> klass->device_plugged(qbus->parent, &local_err); > > > >> } > > > >> @@ -82,9 +78,15 @@ 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) { > > > > > > > > Hi Pasic, > > > > > > > > When testing the virtio-fs in Intel TDX, I met the error report in this > > > > check. Is it appropriate to compare the dma_as against the > > > > address_space_memory to detect whether the IOMMU is enabled or not? Per > > > > the commit ae4003738f(vhost: correctly detect the enabling IOMMU), we > > > > should call virtio_bus_device_iommu_enabled(vdev) instead here, correct? > > > > > > > > > > Sorry for bothering. > > > > > > Can virtio-fs work properly in AMD SEV? > > > > > > IIUC, If get_dma_as() is implemented and in case of PCI, > > > pci_get_address_space() is used and returns the bus master as. This > > > would fail the check here. > > > > I think the reason is that the viritio-fs is used without vIOMMU but > > ACCESS_PLATFORM. > > > > That's why we need to use virtio_bus_device_iommu_enabled() to allow > > this setup to work. > > > > Thanks > > Do you retract your ack then? Somehow, we need a fix on top. Thanks > > > > > > > >> + error_setg(errp, > > > >> + "iommu_platform=true is not supported by the > > > >> device"); > > > >> + return; > > > >> + } > > > >> } else { > > > >> vdev->dma_as = &address_space_memory; > > > >> } > > > >> > > > >> base-commit: 0d564a3e32ba8494014c67cdd2ebf0fb71860dff > > > > > > > >
On Thu, Apr 28, 2022 at 01:52:46PM +0800, Jason Wang wrote: > On Thu, Apr 28, 2022 at 12:57 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Thu, Apr 28, 2022 at 11:01:10AM +0800, Jason Wang wrote: > > > On Wed, Apr 27, 2022 at 8:25 PM Chenyi Qiang <chenyi.qiang@intel.com> wrote: > > > > > > > > > > > > > > > > On 4/22/2022 3:11 PM, Chenyi Qiang wrote: > > > > > > > > > > > > > > > On 2/7/2022 7:28 PM, Halil Pasic wrote: > > > > >> The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > > > > >> unsupported") claims to fail the device hotplug when iommu_platform > > > > >> is requested, but not supported by the (vhost) device. On the first > > > > >> glance the condition for detecting that situation looks perfect, but > > > > >> because a certain peculiarity of virtio_platform it ain't. > > > > >> > > > > >> In fact the aforementioned commit introduces a regression. It breaks > > > > >> virtio-fs support for Secure Execution, and most likely also for AMD SEV > > > > >> or any other confidential guest scenario that relies encrypted guest > > > > >> memory. The same also applies to any other vhost device that does not > > > > >> support _F_ACCESS_PLATFORM. > > > > >> > > > > >> The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates > > > > >> "device can not access all of the guest RAM" and "iova != gpa, thus > > > > >> device needs to translate iova". > > > > >> > > > > >> Confidential guest technologies currently rely on the device/hypervisor > > > > >> offering _F_ACCESS_PLATFORM, so that, after the feature has been > > > > >> negotiated, the guest grants access to the portions of memory the > > > > >> device needs to see. So in for confidential guests, generally, > > > > >> _F_ACCESS_PLATFORM is about the restricted access to memory, but not > > > > >> about the addresses used being something else than guest physical > > > > >> addresses. > > > > >> > > > > >> This is the very reason for which commit f7ef7e6e3b ("vhost: correctly > > > > >> turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the > > > > >> vhost device that does not need it, because on the vhost interface it > > > > >> only means "I/O address translation is needed". > > > > >> > > > > >> This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on > > > > >> VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the > > > > >> situation when _F_ACCESS_PLATFORM is requested, but no I/O translation > > > > >> by the device, and thus no device capability is needed. In this > > > > >> situation claiming that the device does not support iommu_plattform=on > > > > >> is counter-productive. So let us stop doing that! > > > > >> > > > > >> Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > > > > >> Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com> > > > > >> Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but > > > > >> unsupported") > > > > >> Acked-by: Cornelia Huck <cohuck@redhat.com> > > > > >> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > > > >> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > > > >> Cc: Kevin Wolf <kwolf@redhat.com> > > > > >> Cc: qemu-stable@nongnu.org > > > > >> > > > > >> --- > > > > >> > > > > >> v4->v5: > > > > >> * added back the return; so if somebody were to add code to the end of > > > > >> the function we are still good > > > > >> v3->v4: > > > > >> * Fixed commit message (thanks Connie) > > > > >> * Removed counter-productive initialization (thanks Connie) > > > > >> * Added tags > > > > >> v2->v3: > > > > >> * Caught a bug: I tired to check if vdev has the feature > > > > >> ACCESS_PLATFORM after we have forced it. Moved the check > > > > >> to a better place > > > > >> v1->v2: > > > > >> * Commit message tweaks. Most notably fixed commit SHA (Michael) > > > > >> > > > > >> --- > > > > >> --- > > > > >> hw/virtio/virtio-bus.c | 12 +++++++----- > > > > >> 1 file changed, 7 insertions(+), 5 deletions(-) > > > > >> > > > > >> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > > > > >> index d23db98c56..0f69d1c742 100644 > > > > >> --- a/hw/virtio/virtio-bus.c > > > > >> +++ b/hw/virtio/virtio-bus.c > > > > >> @@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, > > > > >> Error **errp) > > > > >> 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); > > > > >> + bool vdev_has_iommu; > > > > >> Error *local_err = NULL; > > > > >> DPRINTF("%s: plug device.\n", qbus->name); > > > > >> @@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, > > > > >> Error **errp) > > > > >> return; > > > > >> } > > > > >> - if (has_iommu && !virtio_host_has_feature(vdev, > > > > >> VIRTIO_F_IOMMU_PLATFORM)) { > > > > >> - error_setg(errp, "iommu_platform=true is not supported by the > > > > >> device"); > > > > >> - return; > > > > >> - } > > > > >> - > > > > >> if (klass->device_plugged != NULL) { > > > > >> klass->device_plugged(qbus->parent, &local_err); > > > > >> } > > > > >> @@ -82,9 +78,15 @@ 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) { > > > > > > > > > > Hi Pasic, > > > > > > > > > > When testing the virtio-fs in Intel TDX, I met the error report in this > > > > > check. Is it appropriate to compare the dma_as against the > > > > > address_space_memory to detect whether the IOMMU is enabled or not? Per > > > > > the commit ae4003738f(vhost: correctly detect the enabling IOMMU), we > > > > > should call virtio_bus_device_iommu_enabled(vdev) instead here, correct? > > > > > > > > > > > > > Sorry for bothering. > > > > > > > > Can virtio-fs work properly in AMD SEV? > > > > > > > > IIUC, If get_dma_as() is implemented and in case of PCI, > > > > pci_get_address_space() is used and returns the bus master as. This > > > > would fail the check here. > > > > > > I think the reason is that the viritio-fs is used without vIOMMU but > > > ACCESS_PLATFORM. > > > > > > That's why we need to use virtio_bus_device_iommu_enabled() to allow > > > this setup to work. > > > > > > Thanks > > > > Do you retract your ack then? > > Somehow, we need a fix on top. > > Thanks OK ... Halil what is your take here? I'd rather have it all lined up not applied piecemeal ... > > > > > > > > > > >> + error_setg(errp, > > > > >> + "iommu_platform=true is not supported by the > > > > >> device"); > > > > >> + return; > > > > >> + } > > > > >> } else { > > > > >> vdev->dma_as = &address_space_memory; > > > > >> } > > > > >> > > > > >> base-commit: 0d564a3e32ba8494014c67cdd2ebf0fb71860dff > > > > > > > > > > >
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c index d23db98c56..0f69d1c742 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) 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); + bool vdev_has_iommu; Error *local_err = NULL; DPRINTF("%s: plug device.\n", qbus->name); @@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) return; } - if (has_iommu && !virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { - error_setg(errp, "iommu_platform=true is not supported by the device"); - return; - } - if (klass->device_plugged != NULL) { klass->device_plugged(qbus->parent, &local_err); } @@ -82,9 +78,15 @@ 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"); + return; + } } else { vdev->dma_as = &address_space_memory; }
The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but unsupported") claims to fail the device hotplug when iommu_platform is requested, but not supported by the (vhost) device. On the first glance the condition for detecting that situation looks perfect, but because a certain peculiarity of virtio_platform it ain't. In fact the aforementioned commit introduces a regression. It breaks virtio-fs support for Secure Execution, and most likely also for AMD SEV or any other confidential guest scenario that relies encrypted guest memory. The same also applies to any other vhost device that does not support _F_ACCESS_PLATFORM. The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates "device can not access all of the guest RAM" and "iova != gpa, thus device needs to translate iova". Confidential guest technologies currently rely on the device/hypervisor offering _F_ACCESS_PLATFORM, so that, after the feature has been negotiated, the guest grants access to the portions of memory the device needs to see. So in for confidential guests, generally, _F_ACCESS_PLATFORM is about the restricted access to memory, but not about the addresses used being something else than guest physical addresses. This is the very reason for which commit f7ef7e6e3b ("vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the vhost device that does not need it, because on the vhost interface it only means "I/O address translation is needed". This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the situation when _F_ACCESS_PLATFORM is requested, but no I/O translation by the device, and thus no device capability is needed. In this situation claiming that the device does not support iommu_plattform=on is counter-productive. So let us stop doing that! Signed-off-by: Halil Pasic <pasic@linux.ibm.com> Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com> Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but unsupported") Acked-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> Cc: Kevin Wolf <kwolf@redhat.com> Cc: qemu-stable@nongnu.org --- v4->v5: * added back the return; so if somebody were to add code to the end of the function we are still good v3->v4: * Fixed commit message (thanks Connie) * Removed counter-productive initialization (thanks Connie) * Added tags v2->v3: * Caught a bug: I tired to check if vdev has the feature ACCESS_PLATFORM after we have forced it. Moved the check to a better place v1->v2: * Commit message tweaks. Most notably fixed commit SHA (Michael) --- --- hw/virtio/virtio-bus.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) base-commit: 0d564a3e32ba8494014c67cdd2ebf0fb71860dff