> 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.
