>Subject: Re: [PATCH v2 rdma-next 1/5] RDMA/umem: Add API to find best driver >supported page size in an MR > >On Fri, May 03, 2019 at 04:01:27PM +0000, Saleem, Shiraz wrote: >> >Subject: Re: [PATCH v2 rdma-next 1/5] RDMA/umem: Add API to find best >> >driver supported page size in an MR >> > >> >On Fri, May 03, 2019 at 03:22:59PM +0000, Saleem, Shiraz wrote: >> >> >This is because mask shouldn't start as zero - the highest >> >> >possible mask is something like log2_ru(umem length) >> >> > >> >> >ie absent other considerations the page size at the NIC should be >> >> >the size of the umem. >> >> > >> >> >Then we scan the sgl and reduce that value based on the physical >> >> >address layout >> >> > >> >> >Then we reduce it again based on the uvirt vs address difference >> >> > >> >> >Oring a '0' into the mask means that step contributes no restriction. >> >> > >> >> >.. >> >> > >> >> >So I think the algorithm is just off as is, it should be more like >> >> > >> >> > // Page size can't be larger than the length of the MR mask = >> >> >log2_ru(umem length); >> >> > >> >> > // offset into the first SGL for umem->addr pgoff = umem->address >> >> >& PAGE_MASK; va = uvirt_addr; >> >> > >> >> >> >> Did you mean pgoff = umem->address & ~PAGE_MASK? >> > >> >Yes... >> >> OK. Don't we need something like this for zero based VA? >> va = uvirt_addr ? uvirt_addr : umem->addr; > >Every MR is created with an IOVA (here called VA). Before we get here the caller >should figure out the IOVA and it should either be 0 or >umem->address in the cases we implement today *however* it can really >be anything and this code shouldn't care.. > >> Also can we do this for all HW? > >All hardware has to support an arbitary IOVA. > >> Or do we keep the IB_UMEM_VA_BASED_OFFSET flag and OR in the >> dma_addr_end (for first SGL) and dma_addr_start (for last SGL) to the >> mask when the flag is not set? > >I wasn't totally sure what that flag did.. > >If we don't have any drivers that need something different today then I would >focus entirely on the: > > PA = MR_PAGE_TABLE[IOVA/PAGE_SIZE] | (IOVA & PAGE_MASK) > OK. Agreed. Shiraz