> -----Original Message-----
> From: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Sent: Monday, 29 August 2022 15:26
> To: Jason Gunthorpe <jgg@xxxxxxxxxx>; Leon Romanovsky
> <leonro@xxxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx; Linus Walleij <linus.walleij@xxxxxxxxxx>
> Subject: fg[EXTERNAL] [PATCH] 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 to get to
> a (void *).
>
> Cc: linux-rdma@xxxxxxxxxxxxxxx
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> drivers/infiniband/sw/siw/siw_qp_tx.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c
> b/drivers/infiniband/sw/siw/siw_qp_tx.c
> index 1f4e60257700..5c7853ba8831 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);
Thanks, looks OK to me!
>
> return NULL;
> }
> @@ -535,7 +535,17 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx,
> struct socket *s)
> } else {
> u64 va = sge->laddr + sge_off;
Maybe better change va from 'u64' to platform specific
'uintptr_t' ? That would avoid one (uintptr_t) cast here and
also a few lines down during crypto_hash_update()..?
Might also make it easier to keep lines to 80 chars ;)
Thanks Linus!
>
> - page_array[seg] = virt_to_page(va & PAGE_MASK);
> + /*
> + * virt_to_page() takes a (void *) pointer, and
> + * the va being uint64 creates a special
> + * problem here needing a double cast to
> + * resolve the situation: first to (uintptr_t)
> + * to preserve all the 64 bits and from there
> + * 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
> *)(uintptr_t)(va & PAGE_MASK));
> if (do_crc)
> crypto_shash_update(
> c_tx->mpa_crc_hd,
> --
> 2.37.2
