On Fri, Mar 16, 2018 at 06:14:56PM +0100, Peter Krempa wrote:
> On Fri, Mar 16, 2018 at 13:37:54 +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
>
> I'd drop the word IOMMU here.
>
> > presence is enough to successfully start a VM.
> > Luckily, this issue is not serious, since as of yet, libvirt mandates
> > mdevs to be pre-created prior to a domain's launch - if it is,
> > everything does work smoothly, because the parent device will ensure the
> > iommu groups we try to access exist. However, if the mdev didn't exist,
> > one would see the following error:
>
> That is true only if the mdev does not exist (since it creates an
> iommu/isolation group) and also no other iommu groups are present on the
> host.
>
> If there are any other iommu group then qemuHostdevHostSupportsPassthroughVFIO
> will return true and you'll still get the proper error that you report
> below.
>
> > "unsupported configuration: Mediated host device assignment requires VFIO
> > support"
>
> In that case this error would be wrong.
>
> >
> > The error msg above is simply wrong and doesn't even reflect the IOMMU
> > reality, so after applying this patch one would rather hit the following
> > error:
> >
> > "device not found: mediated device '<UUID>' not found"
>
> This is okay as long as you point out that the above case would happen
> only if no other iommu groups were present on the host.
>
> >
> > Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> > ---
> > 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) {
>
> ACK with the reworded commit message
Reworded with a description of the one specific case where the error message
would be wrong without the patch and pushed both patches, thanks for review.
Erik
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
