Hi Jason,
> Subject: Re: [RFC v2 1/8] dma-buf: Add support for map/unmap APIs for
> interconnects
>
> On Sun, Oct 26, 2025 at 09:44:13PM -0700, Vivek Kasireddy wrote:
> > For the map operation, the dma-buf core will create an xarray but
> > the exporter needs to populate it with the interconnect specific
> > addresses. And, similarly for unmap, the exporter is expected to
> > cleanup the individual entries of the xarray.
>
> I don't think we should limit this to xarrays, nor do I think it is a
> great datastructure for what is usually needed here..
One of the goals (as suggested by Christian) is to have a container that
can be used with an iterator. So, instead of creating a new data structure,
I figured using an xarray would make sense here. And, since the entries
of an xarray can be of any type, I think another advantage is that the
dma-buf core only needs to be aware of the xarray but the exporter can
use an interconnect specific type to populate the entries that the importer
would be aware of.
>
> I just posted the patches showing what iommufd needs, and it wants
> something like
>
> struct mapping {
> struct p2p_provider *provider;
> size_t nelms;
> struct phys_vec *phys;
> };
>
> Which is not something that make sense as an xarray.
If we do not want to use an xarray, I guess we can try to generalize the
struct that holds the addresses and any additional info (such as provider).
Would any of the following look OK to you:
struct dma_buf_mapping {
struct phys_vec *phys;
unsigned int nents;
void *map_data;
};
Or
struct dma_buf_ranges {
struct range *ranges;
unsigned int nranges;
void *ranges_data;
};
>
> I think the interconnect should have its own functions for map/unmap,
> ie instead of trying to have them as a commmon
> dma_buf_interconnect_ops do something like
In my current design, the exporter would call the interconnect specific
map/unmap functions from its common map() callback. But I guess I can
try to implement and test your suggestions to see if they are more
robust/elegant.
>
> struct dma_buf_interconnect_ops {
> const char *name;
> bool (*supports_interconnects)(struct dma_buf_attachment *attach,
I have this as part of dma_buf_attach_ops for the importer but I'll explore your
idea in more detail.
> const struct dma_buf_interconnect_match
> *,
> unsigned int num_ics);
> };
>
> struct dma_buf_iov_interconnect_ops {
> struct dma_buf_interconnect_ops ic_ops;
> struct xx *(*map)(struct dma_buf_attachment *attach,
Do we want each specific interconnect to have its own return type for map?
> unsigned int *bar_number,
> size_t *nelms);
> // No unmap for iov
> };
>
> static inline struct xx *dma_buf_iov_map(struct dma_buf_attachment
> *attach,
> unsigned int *bar_number,
> size_t *nelms)
> {
> return container_of(attach->ic_ops, struct dma_buf_iov_interconnect_ops,
> ic_ops)->map(
> attach, bar_number, nelms));
> }
>
> > +/**
> > + * dma_buf_attachment_is_dynamic - check if the importer can handle
> move_notify.
> > + * @attach: the attachment to check
> > + *
> > + * Returns true if a DMA-buf importer has indicated that it can handle
> dmabuf
> > + * location changes through the move_notify callback.
> > + */
> > +static inline bool
> > +dma_buf_attachment_is_dynamic(struct dma_buf_attachment *attach)
> > +{
> > + return !!attach->importer_ops;
> > +}
>
> Why is this in this patch?
I figured it makes sense to limit map/unmap interconnect ops to dynamic
importers (that register a move_notify callback) only. I guess I could move the
above change into a separate patch.
>
> I also think this patch should be second in the series, it makes more
> sense to figure out how to attach with an interconnect then show how
> to map/unmap with that interconnect
>
> Like I'm not sure why this introduces allow_ic?
Ok, I'll move it to the other patch that introduces
dma_buf_match_interconnects().
Thanks,
Vivek
>
> Jason
