On Fri, 2015-11-20 at 11:33 -0500, Laine Stump wrote:
> On 11/14/2015 03:36 AM, Shivaprasad G Bhat wrote:
> > The reprobe needs to be called multiple times for vfio devices for each
> > device in the iommu group in future patch. So split the reprobe into a
> > new function. No functional change.
> >
> > Signed-off-by: Shivaprasad G Bhat <sbhat@xxxxxxxxxxxxxxxxxx>
> > ---
> > src/util/virpci.c | 47 +++++++++++++++++++++++++++++++----------------
> > 1 file changed, 31 insertions(+), 16 deletions(-)
> >
> > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > index 89e69e2..febf100 100644
> > --- a/src/util/virpci.c
> > +++ b/src/util/virpci.c
> > @@ -1099,6 +1099,36 @@ virPCIIsKnownStub(char *driver)
> > }
> >
> > static int
> > +virPCIDeviceReprobeHostDriver(virPCIDevicePtr dev,
> > + const char *driver,
> > + const char *drvdir)
> > +{
> > + char *path = NULL;
> > + int ret = -1;
> > +
> > + /* Trigger a re-probe of the device is not in the stub's dynamic
>
> As long as you're moving the code, s/of/if/
> > + * ID table. If the stub is available, but 'remove_id' isn't
> > + * available, then re-probing would just cause the device to be
> > + * re-bound to the stub.
> > + */
> > + if (driver && !(path = virPCIDriverFile(driver, "remove_id")))
> > + goto cleanup;
> > +
> > + if (!driver || !virFileExists(drvdir) || virFileExists(path)) {
> > + if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) {
> > + virReportSystemError(errno,
> > + _("Failed to trigger a re-probe for PCI device '%s'"),
> > + dev->name);
> > + goto cleanup;
> > + }
> > + }
> > + ret = 0;
> > + cleanup:
> > + VIR_FREE(path);
> > + return ret;
> > +}
> > +
> > +static int
> > virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
> > {
> > int result = -1;
> > @@ -1150,24 +1180,9 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
> > goto cleanup;
> > }
> >
> > - /* Trigger a re-probe of the device is not in the stub's dynamic
> > - * ID table. If the stub is available, but 'remove_id' isn't
> > - * available, then re-probing would just cause the device to be
> > - * re-bound to the stub.
> > - */
> > - VIR_FREE(path);
> > - if (driver && !(path = virPCIDriverFile(driver, "remove_id")))
> > + if (virPCIDeviceReprobeHostDriver(dev, driver, drvdir) < 0)
> > goto cleanup;
> >
> > - if (!driver || !virFileExists(drvdir) || virFileExists(path)) {
> > - if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) {
> > - virReportSystemError(errno,
> > - _("Failed to trigger a re-probe for PCI device '%s'"),
> > - dev->name);
> > - goto cleanup;
> > - }
> > - }
> > -
> > result = 0;
> >
> > cleanup:
> >
>
> Seems safe, but is this really what we want to do? I haven't
> read/understood the remaining patches yet, but this makes it sound like
> what is going to happen is that all of the devices will be unbound from
> vfio-pci immediately, so they are "in limbo", and will then be reprobed
> once all devices are unused (and therefore unbound from vfio-pci).
>
> I think that may be a bit dangerous. Instead, we should leave the
> devices bound to vfio-pci until all of them are unused, and at that
> time, we should unbind them all from vfio-pci, then reprobe them all.
> (again, I may have misunderstood the direction, if so ignore this).
>
> If I am misunderstanding, and unbinding from vfio-pci will also be
> delayed until all devices are unused, then ACK.
Why don't we start making use of the driver_override feature that we've
had in the kernel instead of continuing to hack on the racy
add_id/remove_id stuff? We've already solved the problem in the kernel.
Want to bind a device to vfio-pci:
echo vfio-pci > /sys/bus/pci/devices/<dev>/driver_override
if [ -e /sys/bus/pci/devices/<dev>/driver ]; then
echo <dev> > /sys/bus/pci/devices/<dev>/driver/unbind
fi
echo <dev> > /sys/bus/pci/drivers_probe
To rebind, replace vfio-pci in the first echo with null and repeat the
rest. The affects are limited to a single device, we're not going to
have surprise unbound devices show up bound to the driver, and we don't
race anyone manipulating other devices. Thanks,
Alex
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list