> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Michal Kalderon
>
> > From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> > owner@xxxxxxxxxxxxxxx> On Behalf Of Gal Pressman
> >
> > On 27/06/2019 18:52, Jason Gunthorpe wrote:
> > > On Thu, Jun 27, 2019 at 06:25:37PM +0300, Gal Pressman wrote:
> > >> On 27/06/2019 16:58, Michal Kalderon wrote:
> > >>> Create a common API for adding entries to a xa_mmap.
> > >>> This API can be used by drivers that don't require special mapping
> > >>> for user mapped memory.
> > >>>
> > >>> The code was copied from the efa driver almost as is, just renamed
> > >>> function to be generic and not efa specific.
> > >>
> > >> I don't think we should force the mmap flags to be the same for all
> > drivers..
> > >> Take a look at mlx5 for example:
> > >>
> > >> enum mlx5_ib_mmap_cmd {
> > >> MLX5_IB_MMAP_REGULAR_PAGE = 0,
> > >> MLX5_IB_MMAP_GET_CONTIGUOUS_PAGES = 1,
> > >> MLX5_IB_MMAP_WC_PAGE = 2,
> > >> MLX5_IB_MMAP_NC_PAGE = 3,
> > >> /* 5 is chosen in order to be compatible with old versions of
> > >> libmlx5
> > */
> > >> MLX5_IB_MMAP_CORE_CLOCK = 5,
> > >> MLX5_IB_MMAP_ALLOC_WC = 6,
> > >> MLX5_IB_MMAP_CLOCK_INFO = 7,
> > >> MLX5_IB_MMAP_DEVICE_MEM = 8,
> > >> };
> > >>
> > >> The flags taken from EFA aren't necessarily going to work for other
> > drivers.
> > >> Maybe the flags bits should be opaque to ib_core and leave the
> > >> actual mmap callbacks in the drivers. Not sure how dealloc_ucontext
> > >> is going to work with opaque flags though?
> > >
> > > Yes, the driver will have to take care of masking the flags before
> > > lookup
> > >
> > > We should probably store the struct page * in the
> > > rdma_user_mmap_entry() and use that to key struct page behavior.
> > I don't follow why we need the struct page? How will this work for MMIO?
> >
> > >
> > > Do you think we should go further and provide a generic mmap() that
> > > does the right thing? It would not be hard to provide a callback
> > > that computes the pgprot flags
> >
> > I think a generic mmap with a driver callback to do the actual
> > rdma_user_mmap_io/vm_insert_page/... according to the flags is a good
> > approach.
> >
> > If the flags are opaque to ib_core we'll need another callback to tell
> > the driver when the entries are being freed. This way, if the entry is
> > a DMA page for example (stated by the flags), the driver can free these
> buffers.
>
> I think we'll still end up with a fair amount of duplicated code between a few
> drivers.
> How about an optional driver callback to override the default one ?
> We can start the new flags from a new range that isn't used by the other
> drivers, If the flag is in a "driver-specific" range and a driver callback for mmap
> is provided We'll call that function. We can also store in the
> rdma_user_mmap_entry whether The memory was mapped using driver
> opaque fields or not, and call the driver Free buffer functions accordingly.
Jason,
Seems except Mellanox + hns the mmap flags aren't ABI.
Also, current Mellanox code seems like it won't benefit from
mmap cookie helper functions in any case as the mmap function is very specific and the flags used indicate
the address and not just how to map it.
For most drivers (efa, qedr, siw, cxgb3/4, ocrdma) mmap is called on address received
by kernel in some response. Meaning driver can write anything in the response that will serve as the key / flag.
Other drivers ( i40iw ) have a simple mmap function that doesn't require a mmap database at all.
Current EFA implementation is very generic, the flags just indicate how to map the address, it use
the address as is that is placed in the rdma_user_mmap_entry.
If future enhancements are required we will be able to either enhance the flags to have a driver-specific map
That we can call from rdma_user_mmap, or a driver-specific callback inside the rdma_user_mmap_entry
to get the address to map.
Having said that, I'm of course also fine with only adding the mmap_xa database into core and it's helper functions
For inserting, getting, freeing the entries.
Please let me know which direction to take.
Thanks,
Michal
