On Sun, Mar 10, 2019 at 04:05:25PM +0200, Gal Pressman wrote:
> On 07-Mar-19 20:53, Jason Gunthorpe wrote:
> > On Thu, Mar 07, 2019 at 04:44:32PM +0200, Gal Pressman wrote:
> >> On 06-Mar-19 20:34, Jason Gunthorpe wrote:
> >>> On Wed, Mar 06, 2019 at 05:55:11PM +0200, Gal Pressman wrote:
> >>>> Also, EFA is special in the sense that all sg functions use PAGE_SIZE while EFA
> >>>> needs EFA_PAGE_SIZE. I guess it could be solved by passing that as a parameter,
> >>>> but that'll probably require some changes in existing API functions
> >>>> (__sg_alloc_table_from_pages for example)?
> >>>
> >>> No, you are supposed to build up the sgl in large chunks and then
> >>> fragment it into whatever the HW requires after DMA mapping.
> >>
> >> That's fine, I'll change the stride to PAGE_SIZE.
> >>
> >>>
> >>> The code as written is wrong, as the IOMMU is free to consolidate the
> >>> carefully broken up SGL into a single entry after doing dma_map.
> >>>
> >>> Fragementation for HW page size limitations must be done after
> >>> mapping to use the APIs correctly.
> >>
> >> It does.
> >> When building the chunk list (pbl_chunk_list_create) we iterate each sg element
> >> in EFA_PAGE_SIZE strides.
> >
> > This loop in pbl_chunk_list_create should use Shiraz's stuff, but it
> > is OK to update it after his stuff makes it (noting that if Shiraz is
> > merged first you have to fix it) this.
>
> Alright.
>
> >
> > Mind that the sg offset in that loop is not always zero though.
>
> Right, thanks!
>
> for_each_sg(pages_sgl, sg, sg_dma_cnt, entry) {
> npg_in_sg = sg_dma_len(sg) >> EFA_PAGE_SHIFT;
Don't think the above will compute properly if an offset is present.
> for (i = 0; i < npg_in_sg; i++) {
> cur_chunk_buf[page_idx++] = sg_dma_address(sg) +
> ^
> & ~(EFA_PAGE_SIZE - 1) seems right?
>
> (EFA_PAGE_SIZE * i);
Yes, certainly helpful..
But Shiraz's version is cleaner :)
Jason
