[libvirt] [PATCH 2/2] qemu: hostdev: Don't error out on domain with an mdev when IOMMU is off

Erik Skultety posted 2 patches 7 years, 1 month ago
[libvirt] [PATCH 2/2] qemu: hostdev: Don't error out on domain with an mdev when IOMMU is off
Posted by Erik Skultety 7 years, 1 month ago
Commit b4c2ac8d56 made a false assumption that IOMMU support necessary
for an mdev device to be assigned to a VM. Unlike direct PCI assignment,
IOMMU support is not needed for mediated devices, as the physical parent
device provides the IOMMU isolation, therefore, simply checking for VFIO
presence is enough to successfully start a VM.
Luckily, this issue is not very serious, since as of yet, libvirt
mandates the mdevs to be pre-created prior to a domain's launch, so this
patch will merely change the error the end user is going to see.

Previously:
unsupported configuration: Mediated host device assignment requires VFIO
support

Now:
failed to read device '/sys/bus/mdev/devices/<uuid>/': No such file or
directory

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 src/qemu/qemu_hostdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 73d26f4c6..afe445d4e 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -330,9 +330,14 @@ qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
                                   int nhostdevs)
 {
     virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
-    bool supportsVFIO = qemuHostdevHostSupportsPassthroughVFIO();
+    bool supportsVFIO;
     size_t i;

+    /* Checking for VFIO only is fine with mdev, as IOMMU isolation is achieved
+     * by the physical parent device.
+     */
+    supportsVFIO = virFileExists("/dev/vfio/vfio");
+
     for (i = 0; i < nhostdevs; i++) {
         if (hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
             hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
--
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: hostdev: Don't error out on domain with an mdev when IOMMU is off
Posted by Peter Krempa 7 years, 1 month ago
On Fri, Mar 16, 2018 at 11:50:02 +0100, Erik Skultety wrote:
> Commit b4c2ac8d56 made a false assumption that IOMMU support necessary
> for an mdev device to be assigned to a VM. Unlike direct PCI assignment,
> IOMMU support is not needed for mediated devices, as the physical parent
> device provides the IOMMU isolation, therefore, simply checking for VFIO
> presence is enough to successfully start a VM.
> Luckily, this issue is not very serious, since as of yet, libvirt
> mandates the mdevs to be pre-created prior to a domain's launch, so this
> patch will merely change the error the end user is going to see.
> 
> Previously:
> unsupported configuration: Mediated host device assignment requires VFIO
> support
> 
> Now:
> failed to read device '/sys/bus/mdev/devices/<uuid>/': No such file or
> directory

I'm not sure I understood what you wanted to say in the commit message.

You are not requiring the IOMMU to bepresent for the mediated device
since isolation is inherent. This makes sense.

You are then giving an example of error message what would occur if the
iommu groups are not present on the host.

As a 'fixed' result you provide an error message if the user did not
create a MDEV prior to starting the VM. How is that relevant to this
patch? The VM should start just fine in that case, shouldn't it?

> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/qemu/qemu_hostdev.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index 73d26f4c6..afe445d4e 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -330,9 +330,14 @@ qemuHostdevPrepareMediatedDevices(virQEMUDriverPtr driver,
>                                    int nhostdevs)
>  {
>      virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> -    bool supportsVFIO = qemuHostdevHostSupportsPassthroughVFIO();
> +    bool supportsVFIO;
>      size_t i;
> 
> +    /* Checking for VFIO only is fine with mdev, as IOMMU isolation is achieved
> +     * by the physical parent device.
> +     */
> +    supportsVFIO = virFileExists("/dev/vfio/vfio");
> +
>      for (i = 0; i < nhostdevs; i++) {
>          if (hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
>              hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
> --
> 2.13.6
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: hostdev: Don't error out on domain with an mdev when IOMMU is off
Posted by Erik Skultety 7 years, 1 month ago
On Fri, Mar 16, 2018 at 12:07:50PM +0100, Peter Krempa wrote:
> On Fri, Mar 16, 2018 at 11:50:02 +0100, Erik Skultety wrote:
> > Commit b4c2ac8d56 made a false assumption that IOMMU support necessary
> > for an mdev device to be assigned to a VM. Unlike direct PCI assignment,
> > IOMMU support is not needed for mediated devices, as the physical parent
> > device provides the IOMMU isolation, therefore, simply checking for VFIO
> > presence is enough to successfully start a VM.
> > Luckily, this issue is not very serious, since as of yet, libvirt
> > mandates the mdevs to be pre-created prior to a domain's launch, so this
> > patch will merely change the error the end user is going to see.
> >
> > Previously:
> > unsupported configuration: Mediated host device assignment requires VFIO
> > support
> >
> > Now:
> > failed to read device '/sys/bus/mdev/devices/<uuid>/': No such file or
> > directory
>
> I'm not sure I understood what you wanted to say in the commit message.
>
> You are not requiring the IOMMU to bepresent for the mediated device
> since isolation is inherent. This makes sense.
>
> You are then giving an example of error message what would occur if the
> iommu groups are not present on the host.

I should have been more clear about that. So, if you're referencing a
non-existent mdev within the domain XML, the error you get indicates something
that is not true, but you're right in what you deduced that you won't come
across this weird error if the device you're referencing already exists.
Therefore I mentioned in the patch that the issue isn't serious since
everything works if used correctly, only when you don't you'd experience some
odd messages, that's all, I'll rephrase the commit message.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list