* [PATCH v2 1/2] RDMA/rxe: Treat physical addresses right
@ 2023-03-29 14:23 Linus Walleij
2023-03-29 14:23 ` [PATCH v2 2/2] RDMA/rxe: Pass a pointer to virt_to_page() Linus Walleij
2023-04-02 13:51 ` [PATCH v2 1/2] RDMA/rxe: Treat physical addresses right Zhu Yanjun
0 siblings, 2 replies; 4+ messages in thread
From: Linus Walleij @ 2023-03-29 14:23 UTC (permalink / raw)
To: Jason Gunthorpe, Leon Romanovsky; +Cc: linux-rdma, Linus Walleij, Bob Pearson
Whenever the IB_MR_TYPE_DMA flag is set in imbr.type, the "iova"
(I/O virtual address) is not really a virtual address but a physical
address.
This means that the use of virt_to_page() on these addresses is also
incorrect, this needs to be treated and converted to a page using
the page frame number and pfn_to_page().
Fix up all users in this file.
Fixes: 592627ccbdff ("RDMA/rxe: Replace rxe_map and rxe_phys_buf by xarray")
Cc: Bob Pearson <rpearsonhpe@gmail.com>
Reported-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/linux-rdma/ZB2s3GeaN%2FFBpR5K@nvidia.com/
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- New patch prepended to patch set.
---
drivers/infiniband/sw/rxe/rxe_mr.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index b10aa1580a64..8e8250652f9d 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -279,16 +279,20 @@ static int rxe_mr_copy_xarray(struct rxe_mr *mr, u64 iova, void *addr,
return 0;
}
-static void rxe_mr_copy_dma(struct rxe_mr *mr, u64 iova, void *addr,
+/*
+ * This function is always called with a physical address as parameter,
+ * since DMA only operates on physical addresses.
+ */
+static void rxe_mr_copy_dma(struct rxe_mr *mr, u64 phys, void *addr,
unsigned int length, enum rxe_mr_copy_dir dir)
{
- unsigned int page_offset = iova & (PAGE_SIZE - 1);
+ unsigned int page_offset = phys & (PAGE_SIZE - 1);
unsigned int bytes;
struct page *page;
u8 *va;
while (length) {
- page = virt_to_page(iova & mr->page_mask);
+ page = pfn_to_page(phys >> PAGE_SHIFT);
bytes = min_t(unsigned int, length,
PAGE_SIZE - page_offset);
va = kmap_local_page(page);
@@ -300,7 +304,7 @@ static void rxe_mr_copy_dma(struct rxe_mr *mr, u64 iova, void *addr,
kunmap_local(va);
page_offset = 0;
- iova += bytes;
+ phys += bytes;
addr += bytes;
length -= bytes;
}
@@ -487,8 +491,11 @@ 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);
+ /* In this case iova is a physical address */
+ u64 phys = iova;
+
+ page_offset = phys & (PAGE_SIZE - 1);
+ page = pfn_to_page(phys >> PAGE_SHIFT);
} else {
unsigned long index;
int err;
@@ -544,8 +551,11 @@ 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);
+ /* In this case iova is a physical address */
+ u64 phys = iova;
+
+ page_offset = phys & (PAGE_SIZE - 1);
+ page = pfn_to_page(phys >> PAGE_SHIFT);
} else {
unsigned long index;
int err;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] RDMA/rxe: Pass a pointer to virt_to_page()
2023-03-29 14:23 [PATCH v2 1/2] RDMA/rxe: Treat physical addresses right Linus Walleij
@ 2023-03-29 14:23 ` Linus Walleij
2023-03-29 20:14 ` Bob Pearson
2023-04-02 13:51 ` [PATCH v2 1/2] RDMA/rxe: Treat physical addresses right Zhu Yanjun
1 sibling, 1 reply; 4+ messages in thread
From: Linus Walleij @ 2023-03-29 14:23 UTC (permalink / raw)
To: Jason Gunthorpe, Leon Romanovsky; +Cc: linux-rdma, Linus Walleij
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.
Then we need a second cast to (uintptr_t). This is because
the kernel robot builds this driver also for the PARISC,
yielding the following build error on PARISC when casting
(void *) to virt_to_page():
drivers/infiniband/sw/rxe/rxe_mr.c: In function 'rxe_set_page':
>> drivers/infiniband/sw/rxe/rxe_mr.c:216:42: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
216 | struct page *page = virt_to_page((void *)(iova & mr->page_mask));
| ^
include/asm-generic/memory_model.h:18:46: note: in definition of macro '__pfn_to_page'
18 | #define __pfn_to_page(pfn) (mem_map + ((pfn) - ARCH_PFN_OFFSET))
| ^~~
arch/parisc/include/asm/page.h:179:45: note: in expansion of macro '__pa'
179 | #define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
| ^~~~
drivers/infiniband/sw/rxe/rxe_mr.c:216:29: note: in expansion of macro 'virt_to_page'
216 | struct page *page = virt_to_page((void *)(iova & mr->page_mask));
| ^~~~~~~~~~~~
First: I think this happens because iova is u64 by design and
(void *) on PARISC is sometimes 32 bit.
Second: compilation of the SW RXE driver on PARISC is made possible
because it fulfills depends on INFINIBAND_VIRT_DMA since that is just
def_bool !HIGHMEM and PARISC does not have HIGHMEM.
By first casting iova to (uintptr_t) it is turned into a u32 on PARISC
or any other 32bit system and u64 on any 64BIT system.
Link: https://lore.kernel.org/linux-rdma/202303242000.HmTaa6yB-lkp@intel.com/
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Fix up confusion between virtual and physical addresses found
by Jason in a separate patch.
- Fix up compilation on PARISC by additional cast.
I don't know if this is the right solution, perhaps RDMA should
rather depend on 64BIT if the subsystem is only for 64BIT
systems?
---
drivers/infiniband/sw/rxe/rxe_mr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index 8e8250652f9d..a5efb0575956 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)
{
struct rxe_mr *mr = to_rmr(ibmr);
- struct page *page = virt_to_page(iova & mr->page_mask);
+ struct page *page = virt_to_page((void *)(uintptr_t)(iova & mr->page_mask));
bool persistent = !!(mr->access & IB_ACCESS_FLUSH_PERSISTENT);
int err;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] RDMA/rxe: Pass a pointer to virt_to_page()
2023-03-29 14:23 ` [PATCH v2 2/2] RDMA/rxe: Pass a pointer to virt_to_page() Linus Walleij
@ 2023-03-29 20:14 ` Bob Pearson
0 siblings, 0 replies; 4+ messages in thread
From: Bob Pearson @ 2023-03-29 20:14 UTC (permalink / raw)
To: Linus Walleij, Jason Gunthorpe, Leon Romanovsky; +Cc: linux-rdma
On 3/29/23 09:23, 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.
>
> Then we need a second cast to (uintptr_t). This is because
> the kernel robot builds this driver also for the PARISC,
> yielding the following build error on PARISC when casting
> (void *) to virt_to_page():
>
> drivers/infiniband/sw/rxe/rxe_mr.c: In function 'rxe_set_page':
>>> drivers/infiniband/sw/rxe/rxe_mr.c:216:42: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> 216 | struct page *page = virt_to_page((void *)(iova & mr->page_mask));
> | ^
> include/asm-generic/memory_model.h:18:46: note: in definition of macro '__pfn_to_page'
> 18 | #define __pfn_to_page(pfn) (mem_map + ((pfn) - ARCH_PFN_OFFSET))
> | ^~~
> arch/parisc/include/asm/page.h:179:45: note: in expansion of macro '__pa'
> 179 | #define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> | ^~~~
> drivers/infiniband/sw/rxe/rxe_mr.c:216:29: note: in expansion of macro 'virt_to_page'
> 216 | struct page *page = virt_to_page((void *)(iova & mr->page_mask));
> | ^~~~~~~~~~~~
>
> First: I think this happens because iova is u64 by design and
> (void *) on PARISC is sometimes 32 bit.
>
> Second: compilation of the SW RXE driver on PARISC is made possible
> because it fulfills depends on INFINIBAND_VIRT_DMA since that is just
> def_bool !HIGHMEM and PARISC does not have HIGHMEM.
>
> By first casting iova to (uintptr_t) it is turned into a u32 on PARISC
> or any other 32bit system and u64 on any 64BIT system.
>
> Link: https://lore.kernel.org/linux-rdma/202303242000.HmTaa6yB-lkp@intel.com/
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Fix up confusion between virtual and physical addresses found
> by Jason in a separate patch.
> - Fix up compilation on PARISC by additional cast.
> I don't know if this is the right solution, perhaps RDMA should
> rather depend on 64BIT if the subsystem is only for 64BIT
> systems?
> ---
> drivers/infiniband/sw/rxe/rxe_mr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index 8e8250652f9d..a5efb0575956 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)
> {
> struct rxe_mr *mr = to_rmr(ibmr);
> - struct page *page = virt_to_page(iova & mr->page_mask);
> + struct page *page = virt_to_page((void *)(uintptr_t)(iova & mr->page_mask));
> bool persistent = !!(mr->access & IB_ACCESS_FLUSH_PERSISTENT);
> int err;
>
Linus, Thanks for these. AFAIK these are just fine. I am not aware of any interest in 32 bit
support for the rxe driver. My testing has been limited to IA64 architectures but I would be interested
in going further with emulated hardware.
Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/2] RDMA/rxe: Treat physical addresses right
2023-03-29 14:23 [PATCH v2 1/2] RDMA/rxe: Treat physical addresses right Linus Walleij
2023-03-29 14:23 ` [PATCH v2 2/2] RDMA/rxe: Pass a pointer to virt_to_page() Linus Walleij
@ 2023-04-02 13:51 ` Zhu Yanjun
1 sibling, 0 replies; 4+ messages in thread
From: Zhu Yanjun @ 2023-04-02 13:51 UTC (permalink / raw)
To: Linus Walleij, Jason Gunthorpe, Leon Romanovsky; +Cc: linux-rdma, Bob Pearson
在 2023/3/29 22:23, Linus Walleij 写道:
> Whenever the IB_MR_TYPE_DMA flag is set in imbr.type, the "iova"
> (I/O virtual address) is not really a virtual address but a physical
> address.
>
> This means that the use of virt_to_page() on these addresses is also
> incorrect, this needs to be treated and converted to a page using
> the page frame number and pfn_to_page().
>
> Fix up all users in this file.
It is better to have a summary to these 2 commits.
Anyway, thanks.
Acked-by: Zhu Yanjun <zyjzyj2000@gmail.com>
Zhu Yanjun
>
> Fixes: 592627ccbdff ("RDMA/rxe: Replace rxe_map and rxe_phys_buf by xarray")
> Cc: Bob Pearson <rpearsonhpe@gmail.com>
> Reported-by: Jason Gunthorpe <jgg@nvidia.com>
> Link: https://lore.kernel.org/linux-rdma/ZB2s3GeaN%2FFBpR5K@nvidia.com/
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - New patch prepended to patch set.
> ---
> drivers/infiniband/sw/rxe/rxe_mr.c | 26 ++++++++++++++++++--------
> 1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index b10aa1580a64..8e8250652f9d 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -279,16 +279,20 @@ static int rxe_mr_copy_xarray(struct rxe_mr *mr, u64 iova, void *addr,
> return 0;
> }
>
> -static void rxe_mr_copy_dma(struct rxe_mr *mr, u64 iova, void *addr,
> +/*
> + * This function is always called with a physical address as parameter,
> + * since DMA only operates on physical addresses.
> + */
> +static void rxe_mr_copy_dma(struct rxe_mr *mr, u64 phys, void *addr,
> unsigned int length, enum rxe_mr_copy_dir dir)
> {
> - unsigned int page_offset = iova & (PAGE_SIZE - 1);
> + unsigned int page_offset = phys & (PAGE_SIZE - 1);
> unsigned int bytes;
> struct page *page;
> u8 *va;
>
> while (length) {
> - page = virt_to_page(iova & mr->page_mask);
> + page = pfn_to_page(phys >> PAGE_SHIFT);
> bytes = min_t(unsigned int, length,
> PAGE_SIZE - page_offset);
> va = kmap_local_page(page);
> @@ -300,7 +304,7 @@ static void rxe_mr_copy_dma(struct rxe_mr *mr, u64 iova, void *addr,
>
> kunmap_local(va);
> page_offset = 0;
> - iova += bytes;
> + phys += bytes;
> addr += bytes;
> length -= bytes;
> }
> @@ -487,8 +491,11 @@ 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);
> + /* In this case iova is a physical address */
> + u64 phys = iova;
> +
> + page_offset = phys & (PAGE_SIZE - 1);
> + page = pfn_to_page(phys >> PAGE_SHIFT);
> } else {
> unsigned long index;
> int err;
> @@ -544,8 +551,11 @@ 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);
> + /* In this case iova is a physical address */
> + u64 phys = iova;
> +
> + page_offset = phys & (PAGE_SIZE - 1);
> + page = pfn_to_page(phys >> PAGE_SHIFT);
> } else {
> unsigned long index;
> int err;
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-02 13:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-29 14:23 [PATCH v2 1/2] RDMA/rxe: Treat physical addresses right Linus Walleij
2023-03-29 14:23 ` [PATCH v2 2/2] RDMA/rxe: Pass a pointer to virt_to_page() Linus Walleij
2023-03-29 20:14 ` Bob Pearson
2023-04-02 13:51 ` [PATCH v2 1/2] RDMA/rxe: Treat physical addresses right Zhu Yanjun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).