[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] IOMMU and ATS not supported by vhost-user filesystem.
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH] IOMMU and ATS not supported by vhost-user filesystem. |
Date: |
Wed, 27 Jan 2021 19:54:25 +0000 |
User-agent: |
Mutt/1.14.6 (2020-07-11) |
* Laszlo Ersek (lersek@redhat.com) wrote:
> On 01/27/21 12:19, Stefan Hajnoczi wrote:
> > On Tue, Jan 26, 2021 at 03:23:38PM -0300, lagarcia@linux.ibm.com wrote:
> >> From: Leonardo Garcia <lagarcia@br.ibm.com>
> >>
> >> Currently, as IOMMU and ATS are not supported, if a user mistakenly set
> >> any of them and tries to mount the vhost-user filesystem inside the
> >> guest, whenever the user tries to access the mount point, the system
> >> will hang forever.
> >>
> >> Signed-off-by: Leonardo Garcia <lagarcia@br.ibm.com>
> >> ---
> >> hw/virtio/vhost-user-fs-pci.c | 7 +++++++
> >> hw/virtio/vhost-user-fs.c | 5 +++++
> >> 2 files changed, 12 insertions(+)
> >>
> >> diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> >> index 2ed8492b3f..564d1fd108 100644
> >> --- a/hw/virtio/vhost-user-fs-pci.c
> >> +++ b/hw/virtio/vhost-user-fs-pci.c
> >> @@ -11,6 +11,8 @@
> >> * top-level directory.
> >> */
> >>
> >> +#include "qemu/osdep.h"
> >> +#include "qapi/error.h"
> >> #include "qemu/osdep.h"
> >> #include "hw/qdev-properties.h"
> >> #include "hw/virtio/vhost-user-fs.h"
> >> @@ -45,6 +47,11 @@ static void vhost_user_fs_pci_realize(VirtIOPCIProxy
> >> *vpci_dev, Error **errp)
> >> vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 2;
> >> }
> >>
> >> + if (vpci_dev->flags & VIRTIO_PCI_FLAG_ATS) {
> >> + error_setg(errp, "ATS is currently not supported with
> >> vhost-user-fs-pci");
> >> + return;
> >> + }
> >
> > Why is this check needed in addition to VIRTIO_F_IOMMU_PLATFORM?
> >
> > What needs to be added to support ATS?
> >
> >> +
> >> qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> >> }
> >>
> >> diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
> >> index ac4fc34b36..914d68b3ee 100644
> >> --- a/hw/virtio/vhost-user-fs.c
> >> +++ b/hw/virtio/vhost-user-fs.c
> >> @@ -203,6 +203,11 @@ static void vuf_device_realize(DeviceState *dev,
> >> Error **errp)
> >> return;
> >> }
> >>
> >> + if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
> >> + error_setg(errp, "IOMMU is currently not supported with
> >> vhost-user-fs");
> >> + return;
> >> + }
> >> +
> >> if (!vhost_user_init(&fs->vhost_user, &fs->conf.chardev, errp)) {
> >
> > I thought IOMMU support depends on the vhost-user device backend (e.g.
> > virtiofsd), so the vhost-user backend should participate in advertising
> > this feature.
>
> ... I had the same thought when a few weeks earlier I tried to use
> virtio-fs with an SEV guest (just OVMF), and virtiofsd crashed,
> apparently :)
>
> (I didn't report it because, well, SEV wants to prevent sharing between
> host and guest, and virtio-fs works precisely in the opposite direction;
> so the use case may not have much merit.)
Yeh I'm not expecting that to currently work; I can see some uses, but
it's much more niche.
Dave
> Thanks
> Laszlo
>
> >
> > Perhaps the check should be:
> >
> > ret = vhost_dev_init(&fs->vhost_dev, &fs->vhost_user,
> > VHOST_BACKEND_TYPE_USER, 0);
> > if (ret < 0) {
> > error_setg_errno(errp, -ret, "vhost_dev_init failed");
> > goto err_virtio;
> > }
> > +
> > + if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
> > + !(fs->vhost_dev.hdev_features & (1ull << VIRTIO_F_IOMMU_PLATFORM)))
> > {
> > + error_setg(errp, "IOMMU is not supported by the vhost-user device
> > backend");
> > + goto err_iommu_needed;
> > + }
> >
> > Also, can this logic be made generic for all vhost-user devices? It's
> > not really specific to vhost-user-fs.
> >
> > Stefan
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- Re: [PATCH] IOMMU and ATS not supported by vhost-user filesystem., (continued)
Re: [PATCH] IOMMU and ATS not supported by vhost-user filesystem., Laszlo Ersek, 2021/01/27
- Re: [PATCH] IOMMU and ATS not supported by vhost-user filesystem.,
Dr. David Alan Gilbert <=
Re: [PATCH] IOMMU and ATS not supported by vhost-user filesystem., Leonardo Augusto Guimarães Garcia, 2021/01/28