IB/mthca: Fix and simplify page size calculation in mthca_reg_phys_mr()

In mthca_reg_phys_mr(), we calculate the page size for the HCA
hardware to use to map the buffer list passed in by the consumer.
For example, if the consumer passes in

    [0] addr 0x1000, size 0x1000
    [1] addr 0x2000, size 0x1000

then the algorithm would come up with a page size of 0x2000 and a list
of two pages, at 0x0000 and 0x2000.  Usually, this would work fine
since the memory region would start at an offset of 0x1000 and have a
length of 0x2000.

However, the old code did not take into account the alignment of the
IO virtual address passed in.  For example, if the consumer passed in
a virtual address of 0x6000 for the above, then the offset of 0x1000
would not be used correctly because the page mask of 0x1fff would
result in an offset of 0.

We can fix this quite neatly by making sure that the page shift we use
is no bigger than the first bit where the start of the first buffer
and the IO virtual address differ.  Also, we can further simplify the
code by removing the special case for a single buffer by noticing that
it doesn't matter if we use a page size that is too big.  This allows
the loop to compute the page shift to be replaced with __ffs().

Thanks to Bryan S Rosenburg <rosnbrg@us.ibm.com> for pointing out the
original bug and suggesting several ways to improve this patch.

Signed-off-by: Roland Dreier <rolandd@cisco.com>

diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
index 6bcde1cb9688f9860b8116fb22526553a23d5746..19b7f61cf04c17bfd4d7e2396c01f176194d78ac 100644 (file)
--- a/drivers/infiniband/hw/mthca/mthca_provider.c
+++ b/drivers/infiniband/hw/mthca/mthca_provider.c
@@ -923,17 +923,13 @@ static struct ib_mr *mthca_reg_phys_mr(struct ib_pd       *pd,
        struct mthca_mr *mr;
        u64 *page_list;
        u64 total_size;
-       u64 mask;
+       unsigned long mask;
        int shift;
        int npages;
        int err;
        int i, j, n;
 
-       /* First check that we have enough alignment */
-       if ((*iova_start & ~PAGE_MASK) != (buffer_list[0].addr & ~PAGE_MASK))
-               return ERR_PTR(-EINVAL);
-
-       mask = 0;
+       mask = buffer_list[0].addr ^ *iova_start;
        total_size = 0;
        for (i = 0; i < num_phys_buf; ++i) {
                if (i != 0)
@@ -947,17 +943,7 @@ static struct ib_mr *mthca_reg_phys_mr(struct ib_pd       *pd,
        if (mask & ~PAGE_MASK)
                return ERR_PTR(-EINVAL);
 
-       /* Find largest page shift we can use to cover buffers */
-       for (shift = PAGE_SHIFT; shift < 31; ++shift)
-               if (num_phys_buf > 1) {
-                       if ((1ULL << shift) & mask)
-                               break;
-               } else {
-                       if (1ULL << shift >=
-                           buffer_list[0].size +
-                           (buffer_list[0].addr & ((1ULL << shift) - 1)))
-                               break;
-               }
+       shift = __ffs(mask | 1 << 31);
 
        buffer_list[0].size += buffer_list[0].addr & ((1ULL << shift) - 1);
        buffer_list[0].addr &= ~0ull << shift;