From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 084A7CC6B1A for ; Wed, 14 Oct 2026 08:18:22 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A7C7610FC06; Wed, 14 Oct 2026 08:18:16 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9CE6510FBE8 for ; Wed, 14 Oct 2026 08:17:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1791965825; x=1823501825; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=kOeEltxFRgOCO13ymXDFuHlR9hxv/ON4p7bub78u5Sg=; b=hJS1ZF8f5Vwb/sikQIqPP3/jZ6agfeiMz30Cv/UgM86M6amBtwyPtBLm vV3nOVcSlcN3QU2p/NHUT2hmeCQuBUqHN4AlG3HlubPR7SpB8dAbb8u7P 7SGMv9Jf80F1mG3+B1lfyTrgnr6ngyU3rXvIfzd1GKK3xBAGKe2mnSMBP kBCoTHc6Bo4mS+UGrrC/zf52doSMU24QfQP7tIRrOU66xjdAayDw3tpRx 85ZQfTaGpYn81Fe49KkzdQuWabKsYjNlDF9GWbWGKax4xsqaSovTHPS3t TO2u8NcZTk/m8BONNZKBlmEGhMjsJTDakyAdylIBbnizdB1RWFn5VMx7E A==; X-CSE-ConnectionGUID: /Byi6FNhRZe0cYxMBQ7HIA== X-CSE-MsgGUID: U3cHlEN8SZKzBYo9Sbpf4w== X-IronPort-AV: E=McAfee;i="6800,10657,11934"; a="591112" X-IronPort-AV: E=Sophos;i="6.27,157,1787036400"; d="scan'208";a="591112" Received: from orviesa010.jf.intel.com ([10.64.159.150]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Oct 2026 01:17:05 -0700 X-CSE-ConnectionGUID: io5eigSBS8yuKQx5sMB/mQ== X-CSE-MsgGUID: 0UX70wrmQiG1DDkXjV1OHw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.27,157,1787036400"; d="scan'208";a="1814713" Received: from soc-5cg43972f8.clients.intel.com (HELO [172.28.182.3]) ([172.28.182.3]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Oct 2026 01:17:03 -0700 Message-ID: <1e649a32-fed1-4b3f-9673-52121b42e763@linux.intel.com> Date: Wed, 14 Oct 2026 10:17:00 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 i-g-t 1/1] tests/intel/xe_pmu: Provision VFs inside dynamic subtests To: Gustavo Sousa , igt-dev@lists.freedesktop.org Cc: Kamil Konieczny References: <5553bf027e3bdb9253a18fce5477a42ef10f0b25.1791900914.git.marcin.bernatowicz@linux.intel.com> <87jynla1wo.fsf@intel.com> Content-Language: en-US From: "Bernatowicz, Marcin" In-Reply-To: <87jynla1wo.fsf@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: igt-dev@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development mailing list for IGT GPU Tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" On 10/13/2026 8:30 PM, Gustavo Sousa wrote: > Marcin Bernatowicz writes: > >> VF setup helpers can call igt_assert(), which is not allowed in the outer >> igt_subtest_with_dynamic() scope. Set up VFs in the first dynamic case, >> reuse them across engine cases, and clean up after the group. This keeps >> VF setup out of unrelated subtests. >> >> Assisted-by: Copilot:Claude-Opus-5 >> Signed-off-by: Marcin Bernatowicz >> Cc: Gustavo Sousa >> Cc: Kamil Konieczny >> Reviewed-by: Kamil Konieczny >> --- >> tests/intel/xe_pmu.c | 48 +++++++++++++++++++++++++++++++++----------- >> 1 file changed, 36 insertions(+), 12 deletions(-) >> >> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c >> index d41ebba1f..159a7f8a2 100644 >> --- a/tests/intel/xe_pmu.c >> +++ b/tests/intel/xe_pmu.c >> @@ -117,6 +117,8 @@ static char xe_device[NAME_MAX]; >> static bool autoprobe; >> static int total_exec_quantum; >> static bool has_engine_active_ticks; >> +static unsigned int num_functions; > I feel like num_functions as a name for a global variable is a bit too > generic for what its purpose is. > > Maybe sriov_num_fns (or the longer sriov_num_functions). > > And then, for consistency, sriov_vfs_setup_started? > > Anyway, the patch looks good to me. Thanks for this! > > Reviewed-by: Gustavo Sousa Agreed, num_functions is too generic here. I’ll rename them to sriov_num_functions and sriov_vfs_setup_started. Thanks for the review! > >> +static bool vfs_setup_started; >> >> #define test_each_engine(test, fd, hwe) \ >> igt_subtest_with_dynamic(test) \ >> @@ -999,6 +1001,7 @@ static unsigned int enable_and_provision_vfs(int fd) >> num_vfs = total_vfs > 2 ? 2 : total_vfs; >> >> /* Enable VF's */ >> + vfs_setup_started = true; >> igt_sriov_disable_driver_autoprobe(fd); >> igt_sriov_enable_vfs(fd, num_vfs); >> igt_require(igt_sriov_get_enabled_vfs(fd) == num_vfs); >> @@ -1021,6 +1024,16 @@ static unsigned int enable_and_provision_vfs(int fd) >> return num_vfs; >> } >> >> +static void ensure_vfs_enabled(int fd) >> +{ >> + if (num_functions) >> + return; >> + >> + igt_require_f(!vfs_setup_started, >> + "VF setup failed in an earlier dynamic subtest; see its log\n"); >> + num_functions = enable_and_provision_vfs(fd) + 1; >> +} >> + >> static void unprovision_and_disable_vfs(int fd) >> { >> int ret; >> @@ -1039,6 +1052,16 @@ static void unprovision_and_disable_vfs(int fd) >> igt_sriov_clear_exit_handler(); >> } >> >> +static void cleanup_vfs_setup(int fd) >> +{ >> + if (!vfs_setup_started) >> + return; >> + >> + unprovision_and_disable_vfs(fd); >> + vfs_setup_started = false; >> + num_functions = 0; >> +} >> + >> static void stash_gt_freq(int fd, uint32_t **stash_min, uint32_t **stash_max) >> { >> int max_gt, gt; >> @@ -1190,28 +1213,29 @@ int igt_main() >> } >> >> igt_subtest_group() { >> - unsigned int num_fns; >> - >> - igt_fixture() >> - num_fns = enable_and_provision_vfs(fd) + 1; >> - >> igt_describe("Validate engine activity on all functions"); >> - test_each_engine("all-fn-engine-activity-load", fd, eci) >> - engine_activity_all_fn(fd, eci, num_fns); >> + test_each_engine("all-fn-engine-activity-load", fd, eci) { >> + ensure_vfs_enabled(fd); >> + engine_activity_all_fn(fd, eci, num_functions); >> + } >> >> igt_describe("Validate per-function engine activity"); >> - test_each_engine("fn-engine-activity-load", fd, eci) >> - for (int fn = 0; fn < num_fns; fn++) >> - engine_activity_fn(fd, eci, fn, XE_SRIOV_SCHED_PRIORITY_LOW); >> + test_each_engine("fn-engine-activity-load", fd, eci) { >> + ensure_vfs_enabled(fd); >> + for (int fn = 0; fn < num_functions; fn++) >> + engine_activity_fn(fd, eci, fn, >> + XE_SRIOV_SCHED_PRIORITY_LOW); >> + } >> >> igt_describe("Validate per-function engine activity when sched-if-idle is set"); >> test_each_engine("fn-engine-activity-sched-if-idle", fd, eci) { >> - for (int fn = 0; fn < num_fns; fn++) >> + ensure_vfs_enabled(fd); >> + for (int fn = 0; fn < num_functions; fn++) >> engine_activity_fn(fd, eci, fn, XE_SRIOV_SCHED_PRIORITY_NORMAL); >> } >> >> igt_fixture() >> - unprovision_and_disable_vfs(fd); >> + cleanup_vfs_setup(fd); >> } >> >> igt_subtest_group() { >> -- >> 2.43.0