Re: [PATCH 8/9] vfio/pci: export nvlink2 support into vendor vfio_pci drivers

[Alexey Kardashevskiy]

On 11/03/2021 06:40, Jason Gunthorpe wrote:
> On Thu, Mar 11, 2021 at 01:24:47AM +1100, Alexey Kardashevskiy wrote:
> > On 11/03/2021 00:02, Jason Gunthorpe wrote:
> > > On Wed, Mar 10, 2021 at 02:57:57PM +0200, Max Gurtovoy wrote:
> > >
> > > > > > +    .err_handler        = &vfio_pci_core_err_handlers,
> > > > > > +};
> > > > > > +
> > > > > > +#ifdef CONFIG_VFIO_PCI_DRIVER_COMPAT
> > > > > > +struct pci_driver *get_nvlink2gpu_vfio_pci_driver(struct pci_dev *pdev)
> > > > > > +{
> > > > > > +    if (pci_match_id(nvlink2gpu_vfio_pci_driver.id_table, pdev))
> > > > > > +        return &nvlink2gpu_vfio_pci_driver;
> > > > >
> > > > >
> > > > > Why do we need matching PCI ids here instead of looking at the FDT which
> > > > > will work better?
> > > >
> > > > what is FDT ? any is it better to use it instead of match_id ?
> > >
> > > This is emulating the device_driver match for the pci_driver.
> >
> > No it is not, it is a device tree info which lets to skip the linux PCI
> > discovery part (the firmware does it anyway) but it tells nothing about
> > which drivers to bind.
>
> I mean get_nvlink2gpu_vfio_pci_driver() is emulating the PCI match.
>
> Max added a pci driver for NPU here:
>
> +static struct pci_driver npu2_vfio_pci_driver = {
> +	.name			= "npu2-vfio-pci",
> +	.id_table		= npu2_vfio_pci_table,
> +	.probe			= npu2_vfio_pci_probe,
>
>
> new userspace should use driver_override with "npu-vfio-pci" as the
> string not "vfio-pci"
>
> The point of the get_npu2_vfio_pci_driver() is only optional
> compatibility to redirect old userspace using "vfio-pci" in the
> driver_override to the now split driver code so userspace doesn't see
> any change in behavior.
>
> If we don't do this then the vfio-pci driver override will disable the
> npu2 special stuff, since Max took it all out of vfio-pci's
> pci_driver.
>
> It is supposed to match exactly the same match table as the pci_driver
> above. We *don't* want different behavior from what the standrd PCI
> driver matcher will do.


This is not a standard PCI driver though and the main vfio-pci won't 
have a list to match ever. IBM NPU PCI id is unlikely to change ever but 
NVIDIA keeps making new devices which work in those P9 boxes, are you 
going to keep adding those ids to nvlink2gpu_vfio_pci_table? btw can the 
id list have only vendor ids and not have device ids?


> Since we don't have any way to mix in FDT discovery to the standard
> PCI driver match it will still attach the npu driver but not enable
> any special support. This seems OK.



--
Alexey