[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supp
From: |
Halil Pasic |
Subject: |
Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported |
Date: |
Thu, 27 Jan 2022 14:28:00 +0100 |
ping^2
Also adding Brijesh and Daniel, as I believe you guys should be
interested in this, and I'm yet to receive review.
@Brijesh, Daniel: Can you confirm that AMD (SEV) and Power are affected
too, and that the fix works for your platforms as well?
Regards,
Halil
On Tue, 25 Jan 2022 11:21:12 +0100
Halil Pasic <pasic@linux.ibm.com> wrote:
> ping
>
> On Mon, 17 Jan 2022 13:02:38 +0100
> 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") for, which fences _F_ACCESS_PLATFORM
> > form 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")
> > Cc: Kevin Wolf <kwolf@redhat.com>
> > Cc: qemu-stable@nongnu.org
> >
> > ---
> >
> > v1->v2:
> > * Commit message tweaks. Most notably fixed commit SHA (Michael)
> >
> > ---
> > hw/virtio/virtio-bus.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> > index d23db98c56..c1578f3de2 100644
> > --- a/hw/virtio/virtio-bus.c
> > +++ b/hw/virtio/virtio-bus.c
> > @@ -69,11 +69,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);
> > }
> > @@ -88,6 +83,12 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error
> > **errp)
> > } else {
> > vdev->dma_as = &address_space_memory;
> > }
> > +
> > + if (has_iommu && vdev->dma_as != &address_space_memory
> > + && !virtio_host_has_feature(vdev,
> > VIRTIO_F_IOMMU_PLATFORM)) {
> > + error_setg(errp, "iommu_platform=true is not supported by the
> > device");
> > + return;
> > + }
> > }
> >
> > /* Reset the virtio_bus */
> >
> > base-commit: 6621441db50d5bae7e34dbd04bf3c57a27a71b32
>
- [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Halil Pasic, 2022/01/17
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Halil Pasic, 2022/01/25
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported,
Halil Pasic <=
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Brijesh Singh, 2022/01/27
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Daniel Henrique Barboza, 2022/01/27
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Halil Pasic, 2022/01/27
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Michael S. Tsirkin, 2022/01/28
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Daniel Henrique Barboza, 2022/01/28
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Halil Pasic, 2022/01/28
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Daniel Henrique Barboza, 2022/01/28
- Re: [PATCH v2 1/1] virtio: fix the condition for iommu_platform not supported, Michael S. Tsirkin, 2022/01/28