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.
