Re: [PATCH] RDMA/rxe: Pass a pointer to virt_to_page()

Jason Gunthorpe
2023-03-24T14:01:05Z

On Fri, Mar 24, 2023 at 11:32:52AM +0100, Linus Walleij wrote:
> Like the other calls in this function virt_to_page() expects
> a pointer, not an integer.
>
> However since many architectures implement virt_to_pfn() as
> a macro, this function becomes polymorphic and accepts both a
> (unsigned long) and a (void *).
>
> Fix this up with an explicit cast.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/infiniband/sw/rxe/rxe_mr.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index b10aa1580a64..5c90d83002f0 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -213,7 +213,7 @@ int rxe_mr_init_fast(int max_pages, struct rxe_mr *mr)
> static int rxe_set_page(struct ib_mr *ibmr, u64 iova)
> {

All these functions have the wrong names, they are kva not IOVA.

> @@ -288,7 +288,7 @@ static void rxe_mr_copy_dma(struct rxe_mr *mr, u64 iova, void *addr,
> u8 *va;

> while (length) {
> - page = virt_to_page(iova & mr->page_mask);
> + page = virt_to_page((void *)(iova & mr->page_mask));
> bytes = min_t(unsigned int, length,
> PAGE_SIZE - page_offset);

This is actually a bug, this function is only called on IB_MR_TYPE_DMA
and in that case 'iova' is actually a phys addr

So iova should be called phys and the above should be:

page = pfn_to_page(phys >> PAGE_SHIFT);

> @@ -488,7 +488,7 @@ int rxe_mr_do_atomic_op(struct rxe_mr *mr, u64 iova, int opcode,
>
> if (mr->ibmr.type == IB_MR_TYPE_DMA) {
> page_offset = iova & (PAGE_SIZE - 1);
> - page = virt_to_page(iova & PAGE_MASK);
> + page = virt_to_page((void *)(iova & PAGE_MASK));

These masks against PAGE_MASK are not needed, virt_to_page does them
already.

> } else {
> unsigned long index;
> int err;
> @@ -545,7 +545,7 @@ int rxe_mr_do_atomic_write(struct rxe_mr *mr, u64 iova, u64 value)
>
> if (mr->ibmr.type == IB_MR_TYPE_DMA) {
> page_offset = iova & (PAGE_SIZE - 1);
> - page = virt_to_page(iova & PAGE_MASK);
> + page = virt_to_page((void *)(iova & PAGE_MASK));

pfn_to_page here as well

Jason