On Thu, Jul 11, 2019 at 17:53:52 +0200, Michal Privoznik wrote:
> Newer kernels (v3.16-rc1~29^2~6^4) have 'driver_override' file
> which simplifies way of binding a PCI device to desired driver.
> Libvirt has support for this for some time too (v2.3.0-rc1~236),
> but not our virpcimock. So far we did not care because our code
> is designed to deal with this situation. Except for one.
> hypothetical case: binding a device to the vfio-pci driver can be
> successful only via driver_override. Any attempt to bind a PCI
> device to vfio-pci driver using old method (new_id + unbind +
> bind) will fail because of b803b29c1a5. While on vanilla kernel
You've reverted the mentioned commit just before this patch.
Also I did not understand what this is supposed to do from the commit
message. Perhaps due to my limited understanding of the pci detachment
code. So either somebody else reviews this or you'll need to reprhase
the commit message.
> I'm able to use the old method successfully, it's failing on RHEL
> kernels (not sure why).
This does not inspire confidence.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> tests/virpcimock.c | 57 +++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/tests/virpcimock.c b/tests/virpcimock.c
> index 6865f992dc..18d06d11d4 100644
> --- a/tests/virpcimock.c
> +++ b/tests/virpcimock.c
> @@ -87,6 +87,11 @@ char *fakesysfspcidir;
> * Probe for a driver that handles the specified device.
> * Data in format "DDDD:BB:DD.F" (Domain:Bus:Device.Function).
> *
> + * /sys/bus/pci/devices/<device>/driver_override
> + * Name of a driver that overrides preferred driver can be written
> + * here. The device will be attached to it on drivers_probe event.
> + * Writing an empty string (or "\n") clears the override.
> + *
> * As a little hack, we are not mocking write to these files, but close()
> * instead. The advantage is we don't need any self growing array to hold the
> * partial writes and construct them back. We can let all the writes finish,
> @@ -147,6 +152,7 @@ static struct pciDevice *pci_device_find_by_content(const char *path);
> static void pci_driver_new(const char *name, int fail, ...);
> static struct pciDriver *pci_driver_find_by_dev(struct pciDevice *dev);
> static struct pciDriver *pci_driver_find_by_path(const char *path);
> +static struct pciDriver *pci_driver_find_by_driver_override(struct pciDevice *dev);
> static int pci_driver_bind(struct pciDriver *driver, struct pciDevice *dev);
> static int pci_driver_unbind(struct pciDriver *driver, struct pciDevice *dev);
> static int pci_driver_handle_change(int fd, const char *path);
> @@ -202,7 +208,8 @@ make_symlink(const char *path,
> static int
> pci_read_file(const char *path,
> char *buf,
> - size_t buf_size)
> + size_t buf_size,
> + bool truncate)
> {
> int ret = -1;
> int fd = -1;
> @@ -224,7 +231,8 @@ pci_read_file(const char *path,
> goto cleanup;
> }
>
> - if (ftruncate(fd, 0) < 0)
> + if (truncate &&
> + ftruncate(fd, 0) < 0)
> goto cleanup;
>
> ret = 0;
> @@ -398,6 +406,8 @@ pci_device_new_from_stub(const struct pciDevice *data)
> ABORT("@tmp overflow");
> make_file(devpath, "class", tmp, -1);
>
> + make_file(devpath, "driver_override", NULL, -1);
> +
> if (snprintf(tmp, sizeof(tmp),
> "%s/../../../kernel/iommu_groups/%d",
> devpath, dev->iommuGroup) < 0) {
> @@ -441,7 +451,7 @@ pci_device_find_by_content(const char *path)
> {
> char tmp[32];
>
> - if (pci_read_file(path, tmp, sizeof(tmp)) < 0)
> + if (pci_read_file(path, tmp, sizeof(tmp), true) < 0)
> return NULL;
>
> return pci_device_find_by_id(tmp);
> @@ -450,7 +460,10 @@ pci_device_find_by_content(const char *path)
> static int
> pci_device_autobind(struct pciDevice *dev)
> {
> - struct pciDriver *driver = pci_driver_find_by_dev(dev);
> + struct pciDriver *driver = pci_driver_find_by_driver_override(dev);
> +
> + if (!driver)
> + driver = pci_driver_find_by_dev(dev);
>
> if (!driver) {
> /* No driver found. Nothing to do */
> @@ -544,6 +557,36 @@ pci_driver_find_by_path(const char *path)
> return NULL;
> }
>
> +static struct pciDriver *
> +pci_driver_find_by_driver_override(struct pciDevice *dev)
> +{
> + struct pciDriver *ret = NULL;
> + char *path = NULL;
> + char tmp[32];
> + size_t i;
> +
> + if (virAsprintfQuiet(&path,
> + SYSFS_PCI_PREFIX "devices/%s/driver_override",
> + dev->id) < 0)
> + return NULL;
> +
> + if (pci_read_file(path, tmp, sizeof(tmp), false) < 0)
> + goto cleanup;
> +
> + for (i = 0; i < nPCIDrivers; i++) {
> + struct pciDriver *driver = pciDrivers[i];
> +
> + if (STREQ(tmp, driver->name)) {
> + ret = driver;
> + break;
> + }
> + }
> +
> + cleanup:
> + VIR_FREE(path);
VIR_AUTOFREE should be available.
> + return ret;
> +}
> +
> static int
> pci_driver_bind(struct pciDriver *driver,
> struct pciDevice *dev)
> @@ -657,6 +700,8 @@ pci_driver_handle_change(int fd ATTRIBUTE_UNUSED, const char *path)
> ret = pci_driver_handle_remove_id(path);
> else if (STREQ(file, "drivers_probe"))
> ret = pci_driver_handle_drivers_probe(path);
> + else if (STREQ(file, "driver_override"))
> + ret = 0; /* nada */
> else
> ABORT("Not handled write to: %s", path);
> return ret;
> @@ -711,7 +756,7 @@ pci_driver_handle_new_id(const char *path)
> goto cleanup;
> }
>
> - if (pci_read_file(path, buf, sizeof(buf)) < 0)
> + if (pci_read_file(path, buf, sizeof(buf), true) < 0)
> goto cleanup;
>
> if (sscanf(buf, "%x %x", &vendor, &device) < 2) {
> @@ -766,7 +811,7 @@ pci_driver_handle_remove_id(const char *path)
> goto cleanup;
> }
>
> - if (pci_read_file(path, buf, sizeof(buf)) < 0)
> + if (pci_read_file(path, buf, sizeof(buf), true) < 0)
> goto cleanup;
>
> if (sscanf(buf, "%x %x", &vendor, &device) < 2) {
> --
> 2.21.0
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
