On Wed, 28 Feb 2024 22:34:02 +0800
Xin Zeng <xin.zeng@xxxxxxxxx> wrote:
> +static int
> +qat_vf_vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + struct device *dev = &pdev->dev;
> + struct qat_vf_core_device *qat_vdev;
> + int ret;
> +
> + if (!pci_match_id(id, pdev)) {
> + pci_err(pdev, "Incompatible device, disallowing driver_override\n");
> + return -ENODEV;
> + }
I think the question of whether this is the right thing to do is still
up for debate, but as noted in the thread where I raised the question,
this mechanism doesn't actually work.
The probe callback is passed the matching ID from the set of dynamic
IDs added via new_id, the driver id_table, or pci_device_id_any for a
strictly driver_override match. Any of those would satisfy
pci_match_id().
If we wanted the probe function to exclusively match devices in the
id_table, we should call this as:
if (!pci_match_id(qat_vf_vfio_pci_table, pdev))...
If we wanted to still allow dynamic IDs, the test might be more like:
if (id == &pci_device_id_any)...
Allowing dynamic IDs but failing driver_override requires a slightly
more sophisticated user, but is inconsistent. Do we have any
consensus on this? Thanks,
Alex
> +
> + qat_vdev = vfio_alloc_device(qat_vf_core_device, core_device.vdev, dev, &qat_vf_pci_ops);
> + if (IS_ERR(qat_vdev))
> + return PTR_ERR(qat_vdev);
> +
> + pci_set_drvdata(pdev, &qat_vdev->core_device);
> + ret = vfio_pci_core_register_device(&qat_vdev->core_device);
> + if (ret)
> + goto out_put_device;
> +
> + return 0;
> +
> +out_put_device:
> + vfio_put_device(&qat_vdev->core_device.vdev);
> + return ret;
> +}
