Re: [PATCH i-g-t 5/5] tests/intel/xe_pat: Validate XA App Transient feature

[Matthew Auld <matthew.auld@intel.com>]

On 26/02/2026 04:56, Dandamudi, Priyanka wrote:
> 
> 
>> -----Original Message-----
>> From: Auld, Matthew <matthew.auld@intel.com>
>> Sent: 17 February 2026 06:01 PM
>> To: Dandamudi, Priyanka <priyanka.dandamudi@intel.com>; Kempczynski,
>> Zbigniew <zbigniew.kempczynski@intel.com>; igt-dev@lists.freedesktop.org;
>> Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Subject: Re: [PATCH i-g-t 5/5] tests/intel/xe_pat: Validate XA App Transient
>> feature
>>
>> On 13/02/2026 08:46, priyanka.dandamudi@intel.com wrote:
>>> From: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
>>>
>>> Test the scenarios when media is on/off.
>>> When media is off/on, for xa app transient buffer, data should be
>>> coherent between cpu and gpu. Coherency should be maintained from
>> KMD
>>> side irrespective of media engine on/off.
>>>
>>> Signed-off-by: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
>>
>> + Thomas, in case he has some input here.
>>
>>> ---
>>>    tests/intel/xe_pat.c | 101
>> +++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 101 insertions(+)
>>>
>>> diff --git a/tests/intel/xe_pat.c b/tests/intel/xe_pat.c index
>>> f2dff62a9..30e427634 100644
>>> --- a/tests/intel/xe_pat.c
>>> +++ b/tests/intel/xe_pat.c
>>> @@ -19,16 +19,24 @@
>>>    #include "intel_blt.h"
>>>    #include "intel_mocs.h"
>>>    #include "intel_pat.h"
>>> +#include "igt_configfs.h"
>>> +#include "igt_device.h"
>>> +#include "igt_fs.h"
>>> +#include "igt_kmod.h"
>>> +#include "igt_sysfs.h"
>>>    #include "linux_scaffold.h"
>>>
>>>    #include "xe/xe_ioctl.h"
>>>    #include "xe/xe_query.h"
>>>    #include "xe/xe_util.h"
>>> +#include "xe/xe_gt.h"
>>>
>>>    #define XE_COH_NONE          1
>>>    #define XE_COH_AT_LEAST_1WAY 2
>>>
>>>    static bool do_slow_check;
>>> +static char bus_addr[NAME_MAX];
>>> +static struct pci_device *pci_dev;
>>>
>>>    static uint32_t create_object(int fd, int r, int size, uint16_t coh_mode,
>>>    			      bool force_cpu_wc);
>>> @@ -1118,6 +1126,11 @@ const struct pat_index_entry
>> bmg_g21_pat_index_modes[] = {
>>>    	{ NULL, 27, false, "c2-2way",     XE_COH_AT_LEAST_1WAY       },
>>>    };
>>>
>>> +const struct pat_index_entry xe3p_lpg_coherency_pat_index_modes[] = {
>>> +	{ NULL, 18, false, "xa-l3-uc",	 XE_COH_NONE          },
>>> +	{ NULL, 19, false, "xa-l3-2way", XE_COH_AT_LEAST_1WAY },
>>
>> Both of these are XA, so the string is not really accurate here?
>>
>> Also should we maybe add 2WAY here also?
> @Auld, Matthew But we thought that default is 2way in the lib, here also is it required?

Right, but thinking on this again it might be good to hit 2WAY with 
Media explicitly off, if we don't already.

>>
>>> +};
>>> +
>>>    /*
>>>     * Depending on 2M/1G GTT pages we might trigger different PTE layouts
>> for the
>>>     * PAT bits, so make sure we test with and without huge-pages. Also
>>> ensure we @@ -1182,6 +1195,18 @@ static uint32_t create_object(int fd, int
>> r, int size, uint16_t coh_mode,
>>>     * Description: Check some of the xe2 pat_index modes.
>>>     */
>>>
>>> +/**
>>> + * SUBTEST: xa-app-transient-media-off
>>> + * Test category: functionality test
>>> + * Description: Check some of the xe4-lpg pat_index modes with media off.
>>> + */
>>> +
>>> +/**
>>> + * SUBTEST:  xa-app-transient-media-on
>>> + * Test category: functionality test
>>> + * Description: Check some of the xe3p-lpg pat_index modes with media
>> on.
>>> + */
>>> +
>>>    static void subtest_pat_index_modes_with_regions(int fd,
>>>    						 const struct pat_index_entry
>> *modes_arr,
>>>    						 int n_modes)
>>> @@ -1495,6 +1520,52 @@ static void false_sharing(int fd)
>>>    	}
>>>    }
>>>
>>> +static void reset(int sig)
>>> +{
>>> +	int configfs_fd;
>>> +
>>> +	igt_kmod_unbind("xe", bus_addr);
>>> +
>>> +	/* Drop all custom configfs settings from subtests */
>>> +	configfs_fd = igt_configfs_open("xe");
>>> +	if (configfs_fd >= 0)
>>> +		igt_fs_remove_dir(configfs_fd, bus_addr);
>>> +	close(configfs_fd);
>>> +
>>> +	/* Bind again a clean driver with no custom settings */
>>> +	igt_kmod_bind("xe", bus_addr);
>>> +}
>>> +
>>> +static void xa_app_transient_test(int configfs_device_fd, bool
>>> +media_on) {
>>> +	int fd, fw_handle, gt;
>>> +
>>> +	igt_kmod_unbind("xe", bus_addr);
>>> +
>>> +	if (media_on)
>>> +		igt_assert(igt_sysfs_set(configfs_device_fd,
>>> +					 "gt_types_allowed",
>> "primary,media"));
>>> +	else
>>> +		igt_assert(igt_sysfs_set(configfs_device_fd,
>>> +					 "gt_types_allowed", "primary"));
>>> +
>>> +	igt_kmod_bind("xe", bus_addr);
>>> +
>>> +	fd = drm_open_driver(DRIVER_XE);
>>> +
>>> +	fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
>>
>> I think comment here to explain that this is to prevent entering rc6 for the
>> duration of the test, which would otherwise invoke full PPC flushes.
> I will update.
>>
>>> +	igt_require(fw_handle >= 0);
>>> +
>>> +	subtest_pat_index_modes_with_regions(fd,
>> xe3p_lpg_coherency_pat_index_modes,
>>> +
>> ARRAY_SIZE(xe3p_lpg_coherency_pat_index_modes));
>>
>> Ok, so this will hammer these indexes accross blt, render and dw (partial
>> cache-lines), including mixing CPU/GPU access.
> @Auld, Matthew Can you elaborate on this. What is the change expected here?

Just thinking out aloud as I was reading it. Nothing to change :)

>>
>>> +
>>> +	/* check status of c state, it should not be in c6 due to forcewake. */
>>> +	xe_for_each_gt(fd, gt)
>>> +		igt_assert(!xe_gt_is_in_c6(fd, gt));
>>> +
>>> +	close(fw_handle);
>>> +}
>>> +
>>>    static int opt_handler(int opt, int opt_index, void *data)
>>>    {
>>>    	switch (opt) {
>>> @@ -1586,6 +1657,36 @@ int igt_main_args("V", NULL, help_str,
>> opt_handler, NULL)
>>>    		false_sharing(fd);
>>>    	}
>>>
>>> +	igt_subtest_group() {
>>> +		int configfs_fd, configfs_device_fd;
>>> +
>>> +		igt_fixture() {
>>> +			igt_require(intel_graphics_ver(dev_id) == IP_VER(35,
>> 10));
>>> +
>>> +			pci_dev = igt_device_get_pci_device(fd);
>>> +			snprintf(bus_addr, sizeof(bus_addr),
>> "%04x:%02x:%02x.%01x",
>>> +				 pci_dev->domain, pci_dev->bus, pci_dev-
>>> dev, pci_dev->func);
>>> +
>>> +			configfs_fd = igt_configfs_open("xe");
>>> +			igt_require(configfs_fd != -1);
>>> +			configfs_device_fd = igt_fs_create_dir(configfs_fd,
>> bus_addr,
>>> +							       S_IRWXU |
>> S_IRGRP | S_IXGRP |
>>> +							       S_IROTH |
>> S_IXOTH);
>>> +			igt_install_exit_handler(reset);
>>> +		}
>>> +
>>> +		igt_subtest_with_dynamic("xa-app-transient-media-off")
>>> +			xa_app_transient_test(configfs_device_fd, false);
>>> +
>>> +		igt_subtest_with_dynamic("xa-app-transient-media-on")
>>> +			xa_app_transient_test(configfs_device_fd, true);
>>
>> And this will repeat the above, one with Media forcibly off, and with it
>> potentially on. Hopefully giving us both paths.
>>
>> Some other angles to potentially consider:
>>
>> 1) userptr. Still TBD on the exact KMD behaviour with allowing non-XA/non-
>> 2WAY, or banning it. If we ban it then we should verify that.
>> If we allow it, then what we could maybe do is submit a GPU workload to fill
>> some userptr, then do something to trigger an invalidation to the CPU
>> mapping, then read from the CPU to check that we see the GPU writes (this
>> would be with Media off). KMD should flush those cachelines upon triggering
>> invalidation.
>>
>> 2) dma-buf. Still TBD on this also. But same as above where we would want to
>> add some validation for this. If this ends up being banned we should validate
>> that somewhere. If allowed then I guess we need something to at least
>> attempt to write to it from the GPU on importer side using non-xa (exported
>> from vgem), then maybe destroy the importer side and read it from the CPU
>> on exporter/native side? In theory it should all be flushed, so we should see
>> the GPU writes. Thomas, any ideas here?
> @Thomas Hellström any ideas here?

With latest version we are leaning more towards requiring 2WAY or 
XA+coh_1way with dma-buf, userptr and SVM.

With that it might be more a case of checking that the invalid combos 
are rejected.

>>
>>> +
>>> +		igt_fixture() {
>>> +			close(configfs_device_fd);
>>> +			close(configfs_fd);
>>> +		}
>>> +	}
>>> +
>>>    	igt_fixture()
>>>    		drm_close_driver(fd);
>>>    }
>