2020-05-06 04:24:12

by Mehta, Sanju

[permalink] [raw]
Subject: [PATCH v3 0/5] ntb perf, ntb tool and ntb-hw improvements

v3:
- Increased ntb_perf command re-try sleep time
- avoid false dma unmap of dst address.

v2: Incorporated improvements suggested by Logan Gunthorpe

Links of the review comments for v3:
1. https://lkml.org/lkml/2020/3/11/981
2. https://lkml.org/lkml/2020/3/10/1827

Logan Gunthorpe (1):
ntb: hw: remove the code that sets the DMA mask

Sanjay R Mehta (4):
ntb_perf: pass correct struct device to dma_alloc_coherent
ntb_tool: pass correct struct device to dma_alloc_coherent
ntb_perf: increase sleep time from one milli sec to one sec
ntb_perf: avoid false dma unmap of destination address

drivers/ntb/hw/amd/ntb_hw_amd.c | 4 ----
drivers/ntb/hw/idt/ntb_hw_idt.c | 6 ------
drivers/ntb/hw/intel/ntb_hw_gen1.c | 4 ----
drivers/ntb/test/ntb_perf.c | 23 ++++++++---------------
drivers/ntb/test/ntb_tool.c | 6 +++---
5 files changed, 11 insertions(+), 32 deletions(-)

--
2.7.4


2020-05-06 04:24:20

by Mehta, Sanju

[permalink] [raw]
Subject: [PATCH v3 1/5] ntb: hw: remove the code that sets the DMA mask

From: Logan Gunthorpe <[email protected]>

This patch removes the code that sets the DMA mask as it no longer
makes sense to do this.

Fixes: 7f46c8b3a552 ("NTB: ntb_tool: Add full multi-port NTB API support")
Signed-off-by: Logan Gunthorpe <[email protected]>
Tested-by: Alexander Fomichev <[email protected]>
Signed-off-by: Sanjay R Mehta <[email protected]>
---
drivers/ntb/hw/amd/ntb_hw_amd.c | 4 ----
drivers/ntb/hw/idt/ntb_hw_idt.c | 6 ------
drivers/ntb/hw/intel/ntb_hw_gen1.c | 4 ----
3 files changed, 14 deletions(-)

diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
index 9e310e1..88e1db6 100644
--- a/drivers/ntb/hw/amd/ntb_hw_amd.c
+++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
@@ -1191,10 +1191,6 @@ static int amd_ntb_init_pci(struct amd_ntb_dev *ndev,
goto err_dma_mask;
dev_warn(&pdev->dev, "Cannot DMA consistent highmem\n");
}
- rc = dma_coerce_mask_and_coherent(&ndev->ntb.dev,
- dma_get_mask(&pdev->dev));
- if (rc)
- goto err_dma_mask;

ndev->self_mmio = pci_iomap(pdev, 0, 0);
if (!ndev->self_mmio) {
diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
index edae523..d54261f 100644
--- a/drivers/ntb/hw/idt/ntb_hw_idt.c
+++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
@@ -2660,12 +2660,6 @@ static int idt_init_pci(struct idt_ntb_dev *ndev)
dev_warn(&pdev->dev,
"Cannot set consistent DMA highmem bit mask\n");
}
- ret = dma_coerce_mask_and_coherent(&ndev->ntb.dev,
- dma_get_mask(&pdev->dev));
- if (ret != 0) {
- dev_err(&pdev->dev, "Failed to set NTB device DMA bit mask\n");
- return ret;
- }

/*
* Enable the device advanced error reporting. It's not critical to
diff --git a/drivers/ntb/hw/intel/ntb_hw_gen1.c b/drivers/ntb/hw/intel/ntb_hw_gen1.c
index bb57ec2..e053012 100644
--- a/drivers/ntb/hw/intel/ntb_hw_gen1.c
+++ b/drivers/ntb/hw/intel/ntb_hw_gen1.c
@@ -1783,10 +1783,6 @@ static int intel_ntb_init_pci(struct intel_ntb_dev *ndev, struct pci_dev *pdev)
goto err_dma_mask;
dev_warn(&pdev->dev, "Cannot DMA consistent highmem\n");
}
- rc = dma_coerce_mask_and_coherent(&ndev->ntb.dev,
- dma_get_mask(&pdev->dev));
- if (rc)
- goto err_dma_mask;

ndev->self_mmio = pci_iomap(pdev, 0, 0);
if (!ndev->self_mmio) {
--
2.7.4

2020-05-06 04:24:34

by Mehta, Sanju

[permalink] [raw]
Subject: [PATCH v3 2/5] ntb_perf: pass correct struct device to dma_alloc_coherent

Currently, ntb->dev is passed to dma_alloc_coherent
and dma_free_coherent calls. The returned dma_addr_t
is the CPU physical address. This works fine as long
as IOMMU is disabled. But when IOMMU is enabled, we
need to make sure that IOVA is returned for dma_addr_t.
So the correct way to achieve this is by changing the
first parameter of dma_alloc_coherent() as ntb->pdev->dev
instead.

Fixes: 5648e56 ("NTB: ntb_perf: Add full multi-port NTB API support")
Signed-off-by: Logan Gunthorpe <[email protected]>
Signed-off-by: Sanjay R Mehta <[email protected]>
Signed-off-by: Arindam Nath <[email protected]>
---
drivers/ntb/test/ntb_perf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index 972f6d9..1c93b9f 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -557,7 +557,7 @@ static void perf_free_inbuf(struct perf_peer *peer)
return;

(void)ntb_mw_clear_trans(peer->perf->ntb, peer->pidx, peer->gidx);
- dma_free_coherent(&peer->perf->ntb->dev, peer->inbuf_size,
+ dma_free_coherent(&peer->perf->ntb->pdev->dev, peer->inbuf_size,
peer->inbuf, peer->inbuf_xlat);
peer->inbuf = NULL;
}
@@ -586,8 +586,9 @@ static int perf_setup_inbuf(struct perf_peer *peer)

perf_free_inbuf(peer);

- peer->inbuf = dma_alloc_coherent(&perf->ntb->dev, peer->inbuf_size,
- &peer->inbuf_xlat, GFP_KERNEL);
+ peer->inbuf = dma_alloc_coherent(&perf->ntb->pdev->dev,
+ peer->inbuf_size, &peer->inbuf_xlat,
+ GFP_KERNEL);
if (!peer->inbuf) {
dev_err(&perf->ntb->dev, "Failed to alloc inbuf of %pa\n",
&peer->inbuf_size);
@@ -1554,4 +1555,3 @@ static void __exit perf_exit(void)
destroy_workqueue(perf_wq);
}
module_exit(perf_exit);
-
--
2.7.4

2020-05-06 04:26:15

by Mehta, Sanju

[permalink] [raw]
Subject: [PATCH v3 5/5] ntb_perf: avoid false dma unmap of destination address

The DMA map and unmap of destination address is already being
done in perf_init_test() and perf_clear_test() functions.
Hence avoiding it by making necessary changes in perf_copy_chunk()
function.

Signed-off-by: Sanjay R Mehta <[email protected]>
Signed-off-by: Arindam Nath <[email protected]>
---
drivers/ntb/test/ntb_perf.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index dbcfdaa..95a0853 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -804,7 +804,7 @@ static int perf_copy_chunk(struct perf_thread *pthr,
dst_vaddr = dst;
dst_dma_addr = peer->dma_dst_addr + (dst_vaddr - vbase);

- unmap = dmaengine_get_unmap_data(dma_dev, 2, GFP_NOWAIT);
+ unmap = dmaengine_get_unmap_data(dma_dev, 1, GFP_NOWAIT);
if (!unmap)
return -ENOMEM;

@@ -817,15 +817,8 @@ static int perf_copy_chunk(struct perf_thread *pthr,
}
unmap->to_cnt = 1;

- unmap->addr[1] = dst_dma_addr;
- if (dma_mapping_error(dma_dev, unmap->addr[1])) {
- ret = -EIO;
- goto err_free_resource;
- }
- unmap->from_cnt = 1;
-
do {
- tx = dmaengine_prep_dma_memcpy(pthr->dma_chan, unmap->addr[1],
+ tx = dmaengine_prep_dma_memcpy(pthr->dma_chan, dst_dma_addr,
unmap->addr[0], len, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!tx)
msleep(DMA_MDELAY);
--
2.7.4

2020-05-06 04:26:52

by Mehta, Sanju

[permalink] [raw]
Subject: [PATCH v3 3/5] ntb_tool: pass correct struct device to dma_alloc_coherent

Currently, ntb->dev is passed to dma_alloc_coherent
and dma_free_coherent calls. The returned dma_addr_t
is the CPU physical address. This works fine as long
as IOMMU is disabled. But when IOMMU is enabled, we
need to make sure that IOVA is returned for dma_addr_t.
So the correct way to achieve this is by changing the
first parameter of dma_alloc_coherent() as ntb->pdev->dev
instead.

Fixes: 5648e56 ("NTB: ntb_perf: Add full multi-port NTB API support")
Signed-off-by: Sanjay R Mehta <[email protected]>
Signed-off-by: Arindam Nath <[email protected]>
---
drivers/ntb/test/ntb_tool.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
index 69da758..9eaeb22 100644
--- a/drivers/ntb/test/ntb_tool.c
+++ b/drivers/ntb/test/ntb_tool.c
@@ -590,7 +590,7 @@ static int tool_setup_mw(struct tool_ctx *tc, int pidx, int widx,
inmw->size = min_t(resource_size_t, req_size, size);
inmw->size = round_up(inmw->size, addr_align);
inmw->size = round_up(inmw->size, size_align);
- inmw->mm_base = dma_alloc_coherent(&tc->ntb->dev, inmw->size,
+ inmw->mm_base = dma_alloc_coherent(&tc->ntb->pdev->dev, inmw->size,
&inmw->dma_base, GFP_KERNEL);
if (!inmw->mm_base)
return -ENOMEM;
@@ -612,7 +612,7 @@ static int tool_setup_mw(struct tool_ctx *tc, int pidx, int widx,
return 0;

err_free_dma:
- dma_free_coherent(&tc->ntb->dev, inmw->size, inmw->mm_base,
+ dma_free_coherent(&tc->ntb->pdev->dev, inmw->size, inmw->mm_base,
inmw->dma_base);
inmw->mm_base = NULL;
inmw->dma_base = 0;
@@ -629,7 +629,7 @@ static void tool_free_mw(struct tool_ctx *tc, int pidx, int widx)

if (inmw->mm_base != NULL) {
ntb_mw_clear_trans(tc->ntb, pidx, widx);
- dma_free_coherent(&tc->ntb->dev, inmw->size,
+ dma_free_coherent(&tc->ntb->pdev->dev, inmw->size,
inmw->mm_base, inmw->dma_base);
}

--
2.7.4

2020-05-06 04:27:18

by Mehta, Sanju

[permalink] [raw]
Subject: [PATCH v3 4/5] ntb_perf: increase sleep time from one milli sec to one sec

After trying to send commands for a maximum of MSG_TRIES
re-tries, link-up fails due to short sleep time(1ms) between
re-tries. Hence increasing the sleep time to one second providing
sufficient time for perf link-up.

Signed-off-by: Sanjay R Mehta <[email protected]>
Signed-off-by: Arindam Nath <[email protected]>
---
drivers/ntb/test/ntb_perf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index 1c93b9f..dbcfdaa 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -101,8 +101,8 @@ MODULE_DESCRIPTION("PCIe NTB Performance Measurement Tool");
#define DMA_MDELAY 10

#define MSG_TRIES 1000
-#define MSG_UDELAY_LOW 1000
-#define MSG_UDELAY_HIGH 2000
+#define MSG_UDELAY_LOW 1000000
+#define MSG_UDELAY_HIGH 2000000

#define PERF_BUF_LEN 1024

--
2.7.4

2020-05-06 15:38:44

by Logan Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ntb perf, ntb tool and ntb-hw improvements



On 2020-05-05 10:21 p.m., Sanjay R Mehta wrote:
> v3:
> - Increased ntb_perf command re-try sleep time
> - avoid false dma unmap of dst address.
>
> v2: Incorporated improvements suggested by Logan Gunthorpe
>
> Links of the review comments for v3:
> 1. https://lkml.org/lkml/2020/3/11/981
> 2. https://lkml.org/lkml/2020/3/10/1827
>
> Logan Gunthorpe (1):
> ntb: hw: remove the code that sets the DMA mask
>
> Sanjay R Mehta (4):
> ntb_perf: pass correct struct device to dma_alloc_coherent
> ntb_tool: pass correct struct device to dma_alloc_coherent
> ntb_perf: increase sleep time from one milli sec to one sec
> ntb_perf: avoid false dma unmap of destination address

Looks good to me. For these 4 patches:

Reviewed-by: Logan Gunthorpe <[email protected]>

Thanks!

Logan

2020-05-31 19:58:52

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] ntb perf, ntb tool and ntb-hw improvements

On Tue, May 05, 2020 at 11:21:47PM -0500, Sanjay R Mehta wrote:
> v3:
> - Increased ntb_perf command re-try sleep time
> - avoid false dma unmap of dst address.
>
> v2: Incorporated improvements suggested by Logan Gunthorpe

Pulled into the ntb branch.

Thanks,
Jon

>
> Links of the review comments for v3:
> 1. https://lkml.org/lkml/2020/3/11/981
> 2. https://lkml.org/lkml/2020/3/10/1827
>
> Logan Gunthorpe (1):
> ntb: hw: remove the code that sets the DMA mask
>
> Sanjay R Mehta (4):
> ntb_perf: pass correct struct device to dma_alloc_coherent
> ntb_tool: pass correct struct device to dma_alloc_coherent
> ntb_perf: increase sleep time from one milli sec to one sec
> ntb_perf: avoid false dma unmap of destination address
>
> drivers/ntb/hw/amd/ntb_hw_amd.c | 4 ----
> drivers/ntb/hw/idt/ntb_hw_idt.c | 6 ------
> drivers/ntb/hw/intel/ntb_hw_gen1.c | 4 ----
> drivers/ntb/test/ntb_perf.c | 23 ++++++++---------------
> drivers/ntb/test/ntb_tool.c | 6 +++---
> 5 files changed, 11 insertions(+), 32 deletions(-)
>
> --
> 2.7.4
>