On Thu, 25 Sep 2025 20:02:36 -0300 Jason Gunthorpe jgg@nvidia.com wrote:
On Thu, Sep 25, 2025 at 04:31:31PM -0600, Alex Williamson wrote:
On Thu, 25 Sep 2025 08:53:08 -0300 Jason Gunthorpe jgg@nvidia.com wrote:
On Thu, Sep 25, 2025 at 10:03:14AM +0300, Leon Romanovsky wrote:
It would at least make sense to me then to store the provider on the vfio_pci_dma_buf object at the time of the get feature call rather than vfio_pci_core_init_dev() though. That would eliminate patch 08/ and the inline #ifdefs.
I'll change it now. If "enable" function goes to be "get" function, we won't need to store anything in vfio_pci_dma_buf too. At the end, we have exactly two lines "provider = priv->vdev->provider[priv->bar];", which can easily be changed to be "provider = pcim_p2pdma_provider(priv->vdev->pdev, priv->bar)"
Not without some kind of locking change. I'd keep the priv->vdev->provider[priv->bar] because setup during probe doesn't need special locking.
Why do we need to store the provider on the vfio_pci_core_device at probe though, we can get it later via pcim_p2pdma_provider().
Because you'd need some new locking to prevent races.
The race is avoided if we simply call pcim_p2pdma_provider() during probe. We don't need to save the returned provider. That's where it seems like pulling the setup out to a separate function would eliminate this annoying BAR# arg.
Besides, the model here should be to call the function once during probe and get back the allocated provider. The fact internally it is kind of nutzo still shouldn't leak out as a property of the ABI.
I would like to remove this weird behavior where it caches things inside the struct device. That's not normal for an API to do that, it is only done for the genalloc path that this doesn't use.
My goal in caching the provider on the vfio p2pdma object was to avoid caching it on the vfio_pci_core_device, but now we're storing it on the struct device, the vfio_pci_core_device, AND the vfio p2pdma object. Given the current state that it's stored on the struct device, I think we only need a setup call during probe (that could be stubbed out rather than #ifdef'd), then cache the provider on the vfio p2pdma object when a dmabuf is configured. Thanks,
Alex