[PATCH v3] RDMA/siw: Pass a pointer to virt_to_page()

[PATCH v3] RDMA/siw: Pass a pointer to virt_to_page()

[Linus Walleij]

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 *).

Fixes: b9be6f18cf9e ("rdma/siw: transmit path")
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- Add Fixes: tag.
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

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

[Leon Romanovsky]

On Fri, Sep 02, 2022 at 11:59:18PM +0200, Linus Walleij wrote:
> 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 *).
> 
> Fixes: b9be6f18cf9e ("rdma/siw: transmit path")
> Cc: linux-rdma@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v2->v3:
> - Add Fixes: tag.
> 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(-)
> 

Thanks, applied.