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

[Jason Gunthorpe <jgg@nvidia.com>]

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