> -----Original Message-----
> From: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Sent: Thursday, 1 September 2022 15:31
> To: Jason Gunthorpe <jgg@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxx>;
> Bernard Metzler <BMT@xxxxxxxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx; Linus Walleij <linus.walleij@xxxxxxxxxx>
> Subject: [EXTERNAL] [PATCH v2] RDMA/siw: Pass a pointer to virt_to_page()
>
> Functions that work on a pointer to virtual memory such as
> virt_to_pfn() and users of that function such as
> virt_to_page() are supposed to pass a pointer to virtual
> memory, ideally a (void *) or other pointer. However since
> many architectures implement virt_to_pfn() as a macro,
> this function becomes polymorphic and accepts both a
> (unsigned long) and a (void *).
>
> If we instead implement a proper virt_to_pfn(void *addr)
> function the following happens (occurred on arch/arm):
>
> drivers/infiniband/sw/siw/siw_qp_tx.c:32:23: warning: incompatible
> integer to pointer conversion passing 'dma_addr_t' (aka 'unsigned int')
> to parameter of type 'const void *' [-Wint-conversion]
> drivers/infiniband/sw/siw/siw_qp_tx.c:32:37: warning: passing argument
> 1 of 'virt_to_pfn' makes pointer from integer without a cast
> [-Wint-conversion]
> drivers/infiniband/sw/siw/siw_qp_tx.c:538:36: warning: incompatible
> integer to pointer conversion passing 'unsigned long long'
> to parameter of type 'const void *' [-Wint-conversion]
>
> Fix this with an explicit cast. In one case where the SIW
> SGE uses an unaligned u64 we need a double cast modifying the
> virtual address (va) to a platform-specific uintptr_t before
> casting to a (void *).
>
> Cc: linux-rdma@xxxxxxxxxxxxxxx
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> ChangeLog v1->v2:
> - Change the local va variable to be uintptr_t, avoiding
> double casts in two spots.
> ---
> drivers/infiniband/sw/siw/siw_qp_tx.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c
> b/drivers/infiniband/sw/siw/siw_qp_tx.c
> index 1f4e60257700..7d47b521070b 100644
> --- a/drivers/infiniband/sw/siw/siw_qp_tx.c
> +++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
> @@ -29,7 +29,7 @@ static struct page *siw_get_pblpage(struct siw_mem *mem,
> u64 addr, int *idx)
> dma_addr_t paddr = siw_pbl_get_buffer(pbl, offset, NULL, idx);
>
> if (paddr)
> - return virt_to_page(paddr);
> + return virt_to_page((void *)paddr);
>
> return NULL;
> }
> @@ -533,13 +533,23 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx,
> struct socket *s)
> kunmap_local(kaddr);
> }
> } else {
> - u64 va = sge->laddr + sge_off;
> + /*
> + * Cast to an uintptr_t to preserve all 64 bits
> + * in sge->laddr.
> + */
> + uintptr_t va = (uintptr_t)(sge->laddr + sge_off);
>
> - page_array[seg] = virt_to_page(va & PAGE_MASK);
> + /*
> + * virt_to_page() takes a (void *) pointer
> + * so cast to a (void *) meaning it will be 64
> + * bits on a 64 bit platform and 32 bits on a
> + * 32 bit platform.
> + */
> + page_array[seg] = virt_to_page((void *)(va &
> PAGE_MASK));
> if (do_crc)
> crypto_shash_update(
> c_tx->mpa_crc_hd,
> - (void *)(uintptr_t)va,
> + (void *)va,
> plen);
> }
>
> --
> 2.37.2
Thanks Linus! Code looks good to me. For a decent patch,
add a Fixes line and resend. It fixes commit
b9be6f18cf9e rdma/siw: transmit path.
I also found the description of what your patch does
a little to verbose, but that is maybe just my taste,
preferring a very crisp and short description ;)
Thanks,
Bernard.
