Re: [PATCH i-g-t] xe: Add test to check pci memory barrier capability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tejas,
On 2024-10-09 at 15:26:08 +0530, Tejas Upadhyay wrote:

one more nit, imho a patch with new test should have in subject

tests/intel: Add xe_pci_membarrier test

Also see nit about a test name.

> We want to make sure that direct mmap mapping of physical
> page at doorbell space and whole page is accessible in order
> to use pci memory barrier effect effectively.
> 
> This is basic pci memory barrier test to showcase xe driver
> support for feature. In follow up patches we will have more
> of corner and negative tests added later.
> 
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay@xxxxxxxxx>
> ---
>  include/drm-uapi/xe_drm.h       |  3 ++
>  tests/intel/xe_pci_membarrier.c | 77 +++++++++++++++++++++++++++++++++
>  tests/meson.build               |  1 +
>  3 files changed, 81 insertions(+)
>  create mode 100644 tests/intel/xe_pci_membarrier.c
> 
> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> index f0a450db9..866dc8060 100644
> --- a/include/drm-uapi/xe_drm.h
> +++ b/include/drm-uapi/xe_drm.h
> @@ -823,6 +823,9 @@ struct drm_xe_gem_mmap_offset {
>  	/** @offset: The fake offset to use for subsequent mmap call */
>  	__u64 offset;
>  
> +	/* Specific MMAP offset for PCI memory barrier */
> +#define DRM_XE_PCI_BARRIER_MMAP_OFFSET (0x50 << PAGE_SHIFT)
> +
>  	/** @reserved: Reserved */
>  	__u64 reserved[2];
>  };
> diff --git a/tests/intel/xe_pci_membarrier.c b/tests/intel/xe_pci_membarrier.c
> new file mode 100644
> index 000000000..a28a01d4b
> --- /dev/null
> +++ b/tests/intel/xe_pci_membarrier.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright(c) 2024 Intel Corporation. All rights reserved.
> + */
> +
> +#include "xe_drm.h"
> +#include "igt.h"
> +
> +/**
> + * TEST: Test if the driver is capable of putting pci memory barrier using mmap
> + * Category: Core
> + * Mega feature: General Core features
> + * Sub-category: Memory management tests
> + * Functionality: mmap with pre-defined offset
------^^^^^^^^^^^^^
Not sure about this, +cc Katarzyna
Cc: Katarzyna Piecielska <katarzyna.piecielska@xxxxxxxxx>

> + */
> +
> +IGT_TEST_DESCRIPTION("Basic MMAP tests pci memory barrier effect with special offset");
> +#define PAGE_SHIFT 12
> +#define PAGE_SIZE 4096
> +
> +/**
> + * SUBTEST: pci-membarrier-basic

Why not just basic?

> + * Description: create pci memory barrier with write on defined mmap offset.
> + * Test category: functionality test
> + *
> + */
> +
> +static void pci_membarrier(int xe)
> +{
> +	uint64_t flags = MAP_SHARED;
> +	uint64_t offset = DRM_XE_PCI_BARRIER_MMAP_OFFSET;
> +	unsigned int prot = PROT_WRITE;
> +	uint32_t *ptr;
> +	uint64_t size = PAGE_SIZE;
> +	struct timespec tv;
> +
> +	ptr = mmap(NULL, size, prot, flags, xe, offset);
> +	igt_assert(ptr != MAP_FAILED);
> +
> +	/* Check whole page for any errors, also check as
> +	 * we should not read written values back
> +	 */
> +	for (int i = 0; i < size / sizeof(*ptr); i++) {
> +		/* It is expected unconfigured doorbell space
> +		 * will return read value 0xdeadbeef
> +		 */
> +		igt_assert_eq_u32(READ_ONCE(ptr[i]), 0xdeadbeef);
> +
> +		igt_gettime(&tv);
> +		ptr[i] = i;
> +		if (READ_ONCE(ptr[i]) == i) {
> +			while (READ_ONCE(ptr[i]) == i)
> +				;

What if this while never break?
imho use igt_until_timeout around 'for' loop.

> +			igt_info("fd:%d value retained for %"PRId64"ns pos:%d\n",
> +				 xe, igt_nsec_elapsed(&tv), i);

igt_debug is preferred unless this print should never happen,
but even then please limit number of prints.

> +		}
> +		igt_assert_neq(READ_ONCE(ptr[i]), i);
> +	}
> +
> +	munmap(ptr, size);
> +}
> +
> +igt_main
> +{
> +	int xe;
> +
> +	igt_fixture {
> +		xe = drm_open_driver(DRIVER_XE);
> +	}
> +
> +	igt_subtest_f("pci-membarrier-basic")
> +		pci_membarrier(xe);
> +
> +	igt_fixture {
> +		close(xe);

drm_close_driver()

Regards,
Kamil


> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index e5d8852f3..310ef0e0d 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -304,6 +304,7 @@ intel_xe_progs = [
>  	'xe_noexec_ping_pong',
>  	'xe_oa',
>  	'xe_pat',
> +        'xe_pci_membarrier',
>  	'xe_peer2peer',
>  	'xe_pm',
>  	'xe_pm_residency',
> -- 
> 2.34.1
> 



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux