<br><tt><font size=2>Roland Dreier <rdreier@cisco.com> wrote on 01/26/2008
12:04:21 AM:<br>
>  > Incidentally, I've been tracking down exactly the bug that
Steve fixed, <br>
>  > but in mthca_reg_phys_mr() rather than in the cxgb3 <br>
>  > build_phys_page_list().  I'll submit a patch for mthca,
unless someone <br>
>  > else applies Steve's fix there soon.<br>
> <br>
> Strange... is this causing problems in practice?  I do agree
that the<br>
> mthca code, when passed a list like<br>
> <br>
> > 0: addr 0x10001000, size 0x1000<br>
> > 1: addr 0x10002000, size 0x1000<br>
> <br>
> will end up with a page list<br>
> <br>
> > 0: 0x10000000<br>
> > 1: 0x10002000<br>
> <br>
> and a page size of 0x2000, but it seems to me that should work fine
--<br>
> the memory region will end up starting at an offset of 0x1000 and<br>
> having size 0x2000, which gives the region precisely as intended (just<br>
> not in the most obvious way).<br>
</font></tt>
<br><tt><font size=2>Roland, you're quite right that the non-obvious page
list is not necessarily a problem.  It causes a failure only if the
virtual address that is eventually mapped to this region has an alignment
with respect to large-page boundaries that is different from the alignment
of the physical address.  To be concrete, the page list works as you
expect if and only if</font></tt>
<br>
<br><tt><font size=2>    ((*iova_start) & ((1ULL <<
shift) - 1)) ==</font></tt>
<br><tt><font size=2>        (buffer_list[0].addr &
((1ULL << shift) - 1)).</font></tt>
<br>
<br><tt><font size=2>With the example above, the existing algorithm works
fine if, for example, the virtual address is 0x80001000, but it does not
work if the va is 0x80000000.  In the latter case, incoming rdma bytes
destined for va 0x80000000 wind up at physical address 0x10000000, not
at 0x10001000 where they belong.  To answer your first question, yes,
I've seen exactly this behavior in practice.  I don't have a hardware
spec for mthca, but from observation it seems to me that the offset from
the base of the physical region is being derived from the low-order bits
of the virtual address.</font></tt>
<br>
<br><tt><font size=2>So Steve's fix is overly conservative, but it's straightforward
and it has the benefit that it removes the need for the complex special
case for single-element buffer lists.  Before I saw the patch, I was
toying with ways to incorporate the va alignment into the shift calculation,
but I'm not sure the complexity is worth it.  Also, I see that in
cxgb3 the code is factored in such a way that the va isn't even available
during the shift calculation.  That's not a problem for mthca, but
there are advantages to keeping things consistent.</font></tt>
<br>
<br><tt><font size=2>For your amusement, the first time I hit this problem,
I happened to have a buffer list in which the pages were adjacent but out
of order:</font></tt>
<br>
<br><tt><font size=2>0: addr 0x10001000, size 0x1000</font></tt>
<br><tt><font size=2>1: addr 0x10000000, size 0x1000</font></tt>
<br>
<br><tt><font size=2>resulting in the following list of 0x2000-byte pages:</font></tt>
<br>
<br><tt><font size=2>0: 0x10000000</font></tt>
<br><tt><font size=2>1: 0x10000000  !!!</font></tt>
<br>
<br><tt><font size=2>To my surprise, even this bizarre page list works
properly if the virtual address is aligned properly.</font></tt>
<br>
<br><tt><font size=2>- Bryan</font></tt>
<br>
<br>
<br>