diff mbox series

[v6,3/4] test: add test case to validate VFIO DMA map/unmap

Message ID 20201217190604.29803-4-ndabilpuram@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series fix issue with partial DMA unmap |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Nithin Dabilpuram Dec. 17, 2020, 7:06 p.m. UTC 
  Test case alloc's system pages and tries to performs a user
DMA map and unmap both partially and fully.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test/meson.build |   1 +
 app/test/test_vfio.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100644 app/test/test_vfio.c

Comments

Nithin Dabilpuram Dec. 17, 2020, 7:10 p.m. UTC | #1 
Hi David Christensen,

Ping. Let me know if this way of allocation from heap is fine with POWER9 system.

On Fri, Dec 18, 2020 at 12:36:03AM +0530, Nithin Dabilpuram wrote:
> Test case alloc's system pages and tries to performs a user
> DMA map and unmap both partially and fully.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  app/test/meson.build |   1 +
>  app/test/test_vfio.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 108 insertions(+)
>  create mode 100644 app/test/test_vfio.c
> 
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 94fd39f..d9eedb6 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -139,6 +139,7 @@ test_sources = files('commands.c',
>  	'test_trace_register.c',
>  	'test_trace_perf.c',
>  	'test_version.c',
> +	'test_vfio.c',
>  	'virtual_pmd.c'
>  )
>  
> diff --git a/app/test/test_vfio.c b/app/test/test_vfio.c
> new file mode 100644
> index 0000000..c35efed
> --- /dev/null
> +++ b/app/test/test_vfio.c
> @@ -0,0 +1,107 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Marvell.
> + */
> +
> +#include <inttypes.h>
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include <rte_common.h>
> +#include <rte_eal.h>
> +#include <rte_eal_paging.h>
> +#include <rte_errno.h>
> +#include <rte_memory.h>
> +#include <rte_vfio.h>
> +
> +#include "test.h"
> +
> +static int
> +test_memory_vfio_dma_map(void)
> +{
> +	uint64_t sz1, sz2, sz = 2 * rte_mem_page_size();
> +	uint64_t unmap1, unmap2;
> +	uint8_t *alloc_mem;
> +	uint8_t *mem;
> +	int ret;
> +
> +	/* Allocate twice size of requirement from heap to align later */
> +	alloc_mem = malloc(sz * 2);
> +	if (!alloc_mem) {
> +		printf("Skipping test as unable to alloc %"PRIx64"B from heap\n",
> +		       sz * 2);
> +		return 1;
> +	}
> +
> +	/* Force page allocation */
> +	memset(alloc_mem, 0, sz * 2);
> +
> +	mem = RTE_PTR_ALIGN(alloc_mem, rte_mem_page_size());
> +
> +	/* map the whole region */
> +	ret = rte_vfio_container_dma_map(RTE_VFIO_DEFAULT_CONTAINER_FD,
> +					 (uintptr_t)mem, (rte_iova_t)mem, sz);
> +	if (ret) {
> +		/* Check if VFIO is not available or no device is probed */
> +		if (rte_errno == ENOTSUP || rte_errno == ENODEV) {
> +			ret = 1;
> +			goto fail;
> +		}
> +		printf("Failed to dma map whole region, ret=%d(%s)\n",
> +		       ret, rte_strerror(rte_errno));
> +		goto fail;
> +	}
> +
> +	unmap1 = (uint64_t)mem + (sz / 2);
> +	sz1 = sz / 2;
> +	unmap2 = (uint64_t)mem;
> +	sz2 = sz / 2;
> +	/* unmap the partial region */
> +	ret = rte_vfio_container_dma_unmap(RTE_VFIO_DEFAULT_CONTAINER_FD,
> +					   unmap1, (rte_iova_t)unmap1, sz1);
> +	if (ret) {
> +		if (rte_errno == ENOTSUP) {
> +			printf("Partial dma unmap not supported\n");
> +			unmap2 = (uint64_t)mem;
> +			sz2 = sz;
> +		} else {
> +			printf("Failed to unmap second half region, ret=%d(%s)\n",
> +			       ret, rte_strerror(rte_errno));
> +			goto fail;
> +		}
> +	}
> +
> +	/* unmap the remaining region */
> +	ret = rte_vfio_container_dma_unmap(RTE_VFIO_DEFAULT_CONTAINER_FD,
> +					   unmap2, (rte_iova_t)unmap2, sz2);
> +	if (ret) {
> +		printf("Failed to unmap remaining region, ret=%d(%s)\n", ret,
> +		       rte_strerror(rte_errno));
> +		goto fail;
> +	}
> +
> +fail:
> +	free(alloc_mem);
> +	return ret;
> +}
> +
> +static int
> +test_vfio(void)
> +{
> +	int ret;
> +
> +	/* test for vfio dma map/unmap */
> +	ret = test_memory_vfio_dma_map();
> +	if (ret == 1) {
> +		printf("VFIO dma map/unmap unsupported\n");
> +	} else if (ret < 0) {
> +		printf("Error vfio dma map/unmap, ret=%d\n", ret);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +REGISTER_TEST_COMMAND(vfio_autotest, test_vfio);
> -- 
> 2.8.4
>
David Christensen Jan. 5, 2021, 7:33 p.m. UTC | #2 
Hey Nithin,

>> +static int
>> +test_memory_vfio_dma_map(void)
>> +{
>> +	uint64_t sz1, sz2, sz = 2 * rte_mem_page_size();
>> +	uint64_t unmap1, unmap2;
>> +	uint8_t *alloc_mem;
>> +	uint8_t *mem;
>> +	int ret;
>> +
>> +	/* Allocate twice size of requirement from heap to align later */
>> +	alloc_mem = malloc(sz * 2);
>> +	if (!alloc_mem) {
>> +		printf("Skipping test as unable to alloc %"PRIx64"B from heap\n",
>> +		       sz * 2);
>> +		return 1;
>> +	}
>> +
>> +	/* Force page allocation */
>> +	memset(alloc_mem, 0, sz * 2);
>> +
>> +	mem = RTE_PTR_ALIGN(alloc_mem, rte_mem_page_size());
>> +
>> +	/* map the whole region */
>> +	ret = rte_vfio_container_dma_map(RTE_VFIO_DEFAULT_CONTAINER_FD,
>> +					 (uintptr_t)mem, (rte_iova_t)mem, sz);

I'm not sure how to resolve this patch for POWER systems.  The patch 
currently fails with the error:

EAL:   cannot map vaddr for IOMMU, error 22 (Invalid argument)

The problem is that the size argument (page size of 64KB * 2) is smaller 
than the page size set when the DMA window is created (2MB or 1GB 
depending on system configuration for hugepages), resulting in the 
EINVAL error.  When I tried bumping the sz value up to 2 * 1GB the test 
also failed because the VA address was well outside the DMA window set 
when scanning memseg lists.

Allocating heap memory dynamically through the EAL works since it's 
allocated in hugepage size segments and the EAL attempts to keep VA 
memory addresses contiguous, therefore within the defined DMA window. 
But the downside is that the memory is DMA mapped behind the scenes in 
vfio_mem_event_callback().

Not sure how to get around this without duplicating a lot of the heap 
management code in your test.  Maybe others have a suggestion.

Dave
Nithin Dabilpuram Jan. 6, 2021, 8:40 a.m. UTC | #3 
On Tue, Jan 05, 2021 at 11:33:20AM -0800, David Christensen wrote:
> Hey Nithin,
> 
> > > +static int
> > > +test_memory_vfio_dma_map(void)
> > > +{
> > > +	uint64_t sz1, sz2, sz = 2 * rte_mem_page_size();
> > > +	uint64_t unmap1, unmap2;
> > > +	uint8_t *alloc_mem;
> > > +	uint8_t *mem;
> > > +	int ret;
> > > +
> > > +	/* Allocate twice size of requirement from heap to align later */
> > > +	alloc_mem = malloc(sz * 2);
> > > +	if (!alloc_mem) {
> > > +		printf("Skipping test as unable to alloc %"PRIx64"B from heap\n",
> > > +		       sz * 2);
> > > +		return 1;
> > > +	}
> > > +
> > > +	/* Force page allocation */
> > > +	memset(alloc_mem, 0, sz * 2);
> > > +
> > > +	mem = RTE_PTR_ALIGN(alloc_mem, rte_mem_page_size());
> > > +
> > > +	/* map the whole region */
> > > +	ret = rte_vfio_container_dma_map(RTE_VFIO_DEFAULT_CONTAINER_FD,
> > > +					 (uintptr_t)mem, (rte_iova_t)mem, sz);
> 
> I'm not sure how to resolve this patch for POWER systems.  The patch
> currently fails with the error:
> 
> EAL:   cannot map vaddr for IOMMU, error 22 (Invalid argument)
> 
> The problem is that the size argument (page size of 64KB * 2) is smaller
> than the page size set when the DMA window is created (2MB or 1GB depending
> on system configuration for hugepages), resulting in the EINVAL error.  When
> I tried bumping the sz value up to 2 * 1GB the test also failed because the
> VA address was well outside the DMA window set when scanning memseg lists.
> 
> Allocating heap memory dynamically through the EAL works since it's
> allocated in hugepage size segments and the EAL attempts to keep VA memory
> addresses contiguous, therefore within the defined DMA window. But the
> downside is that the memory is DMA mapped behind the scenes in
> vfio_mem_event_callback().
> 
> Not sure how to get around this without duplicating a lot of the heap
> management code in your test.  Maybe others have a suggestion.

David, Anatoly Burakov,

Given that both malloc'ed memory and mmap'd memory is not working for POWER9
setup, I can either drop this test patch(3/4) alone or restrict it to non-POWER9
system's. Since main fix is already acked, I think it shouldn't be a problem to
drop test case which was only added to demostrate the problem.

Any thoughts ?
> 
> Dave
David Christensen Jan. 6, 2021, 9:20 p.m. UTC | #4 
On 1/6/21 12:40 AM, Nithin Dabilpuram wrote:
> On Tue, Jan 05, 2021 at 11:33:20AM -0800, David Christensen wrote:
>> Hey Nithin,
>>
>>>> +static int
>>>> +test_memory_vfio_dma_map(void)
>>>> +{
>>>> +	uint64_t sz1, sz2, sz = 2 * rte_mem_page_size();
>>>> +	uint64_t unmap1, unmap2;
>>>> +	uint8_t *alloc_mem;
>>>> +	uint8_t *mem;
>>>> +	int ret;
>>>> +
>>>> +	/* Allocate twice size of requirement from heap to align later */
>>>> +	alloc_mem = malloc(sz * 2);
>>>> +	if (!alloc_mem) {
>>>> +		printf("Skipping test as unable to alloc %"PRIx64"B from heap\n",
>>>> +		       sz * 2);
>>>> +		return 1;
>>>> +	}
>>>> +
>>>> +	/* Force page allocation */
>>>> +	memset(alloc_mem, 0, sz * 2);
>>>> +
>>>> +	mem = RTE_PTR_ALIGN(alloc_mem, rte_mem_page_size());
>>>> +
>>>> +	/* map the whole region */
>>>> +	ret = rte_vfio_container_dma_map(RTE_VFIO_DEFAULT_CONTAINER_FD,
>>>> +					 (uintptr_t)mem, (rte_iova_t)mem, sz);
>>
>> I'm not sure how to resolve this patch for POWER systems.  The patch
>> currently fails with the error:
>>
>> EAL:   cannot map vaddr for IOMMU, error 22 (Invalid argument)
>>
>> The problem is that the size argument (page size of 64KB * 2) is smaller
>> than the page size set when the DMA window is created (2MB or 1GB depending
>> on system configuration for hugepages), resulting in the EINVAL error.  When
>> I tried bumping the sz value up to 2 * 1GB the test also failed because the
>> VA address was well outside the DMA window set when scanning memseg lists.
>>
>> Allocating heap memory dynamically through the EAL works since it's
>> allocated in hugepage size segments and the EAL attempts to keep VA memory
>> addresses contiguous, therefore within the defined DMA window. But the
>> downside is that the memory is DMA mapped behind the scenes in
>> vfio_mem_event_callback().
>>
>> Not sure how to get around this without duplicating a lot of the heap
>> management code in your test.  Maybe others have a suggestion.
> 
> David, Anatoly Burakov,
> 
> Given that both malloc'ed memory and mmap'd memory is not working for POWER9
> setup, I can either drop this test patch(3/4) alone or restrict it to non-POWER9
> system's. Since main fix is already acked, I think it shouldn't be a problem to
> drop test case which was only added to demostrate the problem.
> 
> Any thoughts ?

I dislike having to special case the architecture in general but I don't 
see an easy solution in this case. I'd be fine with either option, 
dropping the test case of disabling test case support for all POWER 
architecture.

Dave
diff mbox series

Patch

diff --git a/app/test/meson.build b/app/test/meson.build
index 94fd39f..d9eedb6 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -139,6 +139,7 @@  test_sources = files('commands.c',
 	'test_trace_register.c',
 	'test_trace_perf.c',
 	'test_version.c',
+	'test_vfio.c',
 	'virtual_pmd.c'
 )
 
diff --git a/app/test/test_vfio.c b/app/test/test_vfio.c
new file mode 100644
index 0000000..c35efed
--- /dev/null
+++ b/app/test/test_vfio.c
@@ -0,0 +1,107 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2020 Marvell.
+ */
+
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+#include <rte_common.h>
+#include <rte_eal.h>
+#include <rte_eal_paging.h>
+#include <rte_errno.h>
+#include <rte_memory.h>
+#include <rte_vfio.h>
+
+#include "test.h"
+
+static int
+test_memory_vfio_dma_map(void)
+{
+	uint64_t sz1, sz2, sz = 2 * rte_mem_page_size();
+	uint64_t unmap1, unmap2;
+	uint8_t *alloc_mem;
+	uint8_t *mem;
+	int ret;
+
+	/* Allocate twice size of requirement from heap to align later */
+	alloc_mem = malloc(sz * 2);
+	if (!alloc_mem) {
+		printf("Skipping test as unable to alloc %"PRIx64"B from heap\n",
+		       sz * 2);
+		return 1;
+	}
+
+	/* Force page allocation */
+	memset(alloc_mem, 0, sz * 2);
+
+	mem = RTE_PTR_ALIGN(alloc_mem, rte_mem_page_size());
+
+	/* map the whole region */
+	ret = rte_vfio_container_dma_map(RTE_VFIO_DEFAULT_CONTAINER_FD,
+					 (uintptr_t)mem, (rte_iova_t)mem, sz);
+	if (ret) {
+		/* Check if VFIO is not available or no device is probed */
+		if (rte_errno == ENOTSUP || rte_errno == ENODEV) {
+			ret = 1;
+			goto fail;
+		}
+		printf("Failed to dma map whole region, ret=%d(%s)\n",
+		       ret, rte_strerror(rte_errno));
+		goto fail;
+	}
+
+	unmap1 = (uint64_t)mem + (sz / 2);
+	sz1 = sz / 2;
+	unmap2 = (uint64_t)mem;
+	sz2 = sz / 2;
+	/* unmap the partial region */
+	ret = rte_vfio_container_dma_unmap(RTE_VFIO_DEFAULT_CONTAINER_FD,
+					   unmap1, (rte_iova_t)unmap1, sz1);
+	if (ret) {
+		if (rte_errno == ENOTSUP) {
+			printf("Partial dma unmap not supported\n");
+			unmap2 = (uint64_t)mem;
+			sz2 = sz;
+		} else {
+			printf("Failed to unmap second half region, ret=%d(%s)\n",
+			       ret, rte_strerror(rte_errno));
+			goto fail;
+		}
+	}
+
+	/* unmap the remaining region */
+	ret = rte_vfio_container_dma_unmap(RTE_VFIO_DEFAULT_CONTAINER_FD,
+					   unmap2, (rte_iova_t)unmap2, sz2);
+	if (ret) {
+		printf("Failed to unmap remaining region, ret=%d(%s)\n", ret,
+		       rte_strerror(rte_errno));
+		goto fail;
+	}
+
+fail:
+	free(alloc_mem);
+	return ret;
+}
+
+static int
+test_vfio(void)
+{
+	int ret;
+
+	/* test for vfio dma map/unmap */
+	ret = test_memory_vfio_dma_map();
+	if (ret == 1) {
+		printf("VFIO dma map/unmap unsupported\n");
+	} else if (ret < 0) {
+		printf("Error vfio dma map/unmap, ret=%d\n", ret);
+		return -1;
+	}
+
+	return 0;
+}
+
+REGISTER_TEST_COMMAND(vfio_autotest, test_vfio);