Hi Jason,
> Subject: Re: [RFC v2 2/8] dma-buf: Add a helper to match interconnects
> between exporter/importer
>
> On Sun, Oct 26, 2025 at 09:44:14PM -0700, Vivek Kasireddy wrote:
> > +/**
> > + * dma_buf_match_interconnects - determine if there is a specific
> interconnect
> > + * that is supported by both exporter and importer.
> > + * @attach: [in] attachment to populate ic_match field
> > + * @exp: [in] array of interconnects supported by exporter
> > + * @exp_ics: [in] number of interconnects supported by exporter
> > + * @imp: [in] array of interconnects supported by importer
> > + * @imp_ics: [in] number of interconnects supported by importer
> > + *
> > + * This helper function iterates through the list interconnects supported
> > by
> > + * both exporter and importer to find a match. A successful match means
> that
> > + * a common interconnect type is supported by both parties and the
> exporter's
> > + * match_interconnect() callback also confirms that the importer is
> compatible
> > + * with the exporter for that interconnect type.
>
> Document which of the exporter/importer is supposed to call this
I missed adding that part. The importer is expected to call this in my current
design.
>
> > + *
> > + * If a match is found, the attach->ic_match field is populated with a copy
> > + * of the exporter's match data.
>
> > + * Return: true if a match is found, false otherwise.
> > + */
> > +bool dma_buf_match_interconnects(struct dma_buf_attachment *attach,
> > + const struct dma_buf_interconnect_match
> *exp,
> > + unsigned int exp_ics,
> > + const struct dma_buf_interconnect_match
> *imp,
> > + unsigned int imp_ics)
> > +{
> > + const struct dma_buf_interconnect_ops *ic_ops;
> > + struct dma_buf_interconnect_match *ic_match;
> > + struct dma_buf *dmabuf = attach->dmabuf;
> > + unsigned int i, j;
> > +
> > + if (!exp || !imp)
> > + return false;
> > +
> > + if (!attach->allow_ic)
> > + return false;
>
> Seems redundant with this check for ic_ops == NULL:
Not really; attach->allow_ic would indicate if a successful match is
found or not. And, ic_ops is for the exporter to indicate whether it
supports interconnect ops or not.
>
> > + ic_ops = dmabuf->ops->interconnect_ops;
> > + if (!ic_ops || !ic_ops->match_interconnect)
> > + return false;
>
> This seems like too much of a maze to me..
>
> I think you should structure it like this. First declare an interconnect:
>
> struct dma_buf_interconnect iov_interconnect {
> .name = "IOV interconnect",
> .match =..
> }
>
> Then the exporters "subclass"
>
> struct dma_buf_interconnect_ops vfio_iov_interconnect {
> .interconnect = &iov_interconnect,
> .map = vfio_map,
> }
>
> I guess no container_of technique..
>
> Then in VFIO's attach trigger the new code:
>
> const struct dma_buf_interconnect_match vfio_exp_ics[] = {
> {&vfio_iov_interconnect},
> };
>
> dma_buf_match_interconnects(attach, &vfio_exp_ics))
>
> Which will callback to the importer:
>
> static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = {
> .get_importer_interconnects
> }
>
> dma_buf_match_interconnects() would call
> aops->get_importer_interconnects
> and matchs first on .interconnect, then call the interconnect->match
> function with exp/inpt match structs if not NULL.
Ok, I'll try to test your suggestions.
>
> > +struct dma_buf_interconnect_match {
> > + const struct dma_buf_interconnect *type;
> > + struct device *dev;
> > + unsigned int bar;
> > +};
>
> This should be more general, dev and bar are unique to the iov
> importer. Maybe just simple:
>
> struct dma_buf_interconnect_match {
> struct dma_buf_interconnect *ic; // no need for type
> const struct dma_buf_interconnct_ops *exporter_ic_ops;
> u64 match_data[2]; // dev and bar are IOV specific, generalize
I am wondering what kind of match data would be needed for other
interconnects, so that we can try to generalize dma_buf_interconnect_match
or probably have interconnect specific implementations subclass it.
> };
>
> Then some helper
>
> const struct dma_buf_interconnect_match supports_ics[] = {
> IOV_INTERCONNECT(&vfio_iov_interconnect, dev, bar),
> }
I have done mostly the same thing as you suggest in patches 4 and 5 of this
series that add IOV interconnect support for vfio-pci and Xe drivers. Did you
get a chance to look at them?
>
> And it would be nice if interconnect aware drivers could more easially
> interwork with non-interconnect importers.
>
> So I'd add a exporter type of 'p2p dma mapped scatterlist' that just
> matches the legacy importer.
IIUC, even interconnect aware drivers (or exporters) would need to implement
map_dma_buf() op (which is mandatory) which would return an sg_table.
So, if match_interconnects() fails, then the exporter/importer would need to
fallback to using the legacy path.
Thanks,
Vivek
>
> Jason
