From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 888EE10E942 for ; Thu, 11 Jan 2024 14:45:28 +0000 (UTC) Date: Thu, 11 Jan 2024 09:45:22 -0500 From: Rodrigo Vivi To: Bommu Krishnaiah Subject: Re: [PATCH v3 1/3] tests/xe_waitfence: Rename invalid_engine to invalid-exec_queue Message-ID: References: <20240111062518.393544-1-krishnaiah.bommu@intel.com> <20240111062518.393544-2-krishnaiah.bommu@intel.com> Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240111062518.393544-2-krishnaiah.bommu@intel.com> MIME-Version: 1.0 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Thu, Jan 11, 2024 at 11:55:16AM +0530, Bommu Krishnaiah wrote: > Rename invalid_engine to invalid-exec_queue subtest and changed return value > as per kernel implementation. > > Signed-off-by: Bommu Krishnaiah > Cc: Rodrigo Vivi > --- > tests/intel/xe_waitfence.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tests/intel/xe_waitfence.c b/tests/intel/xe_waitfence.c > index 7ba20764c..fd515a151 100644 > --- a/tests/intel/xe_waitfence.c > +++ b/tests/intel/xe_waitfence.c > @@ -151,8 +151,8 @@ waitfence(int fd, enum waittype wt) > * SUBTEST: invalid-ops > * Description: Check query with invalid ops returns expected error code > * > - * SUBTEST: invalid-engine > - * Description: Check query with invalid engine info returns expected error code > + * SUBTEST: invalid-exec_queue > + * Description: Check query with invalid exec_queue info returns expected error code > * > * SUBTEST: exec_queue-reset-wait > * Description: Don’t wait till timeout on user fence when exec_queue reset is detected and return return proper error > @@ -207,7 +207,7 @@ invalid_ops(int fd) > } > > static void > -invalid_engine(int fd) > +invalid_exec_queue(int fd) > { > uint32_t bo; > > @@ -218,7 +218,7 @@ invalid_engine(int fd) > .value = 1, > .mask = DRM_XE_UFENCE_WAIT_MASK_U64, > .timeout = -1, > - .exec_queue_id = 0, > + .exec_queue_id = -1, The exec_queue_id shouldn't be here. It is totally ignored in the exec_queue_create ioctl that is what is getting tested here. > }; > > uint32_t vm = xe_vm_create(fd, 0, 0); > @@ -227,7 +227,7 @@ invalid_engine(int fd) > > do_bind(fd, vm, bo, 0, 0x200000, 0x40000, 1); > > - do_ioctl_err(fd, DRM_IOCTL_XE_WAIT_USER_FENCE, &wait, EFAULT); > + do_ioctl_err(fd, DRM_IOCTL_XE_WAIT_USER_FENCE, &wait, ENOENT); I'm afraid that this is wrong as well. ENOENT is not returned in the exec_queue_create. It is EFAULT that is returned when the wrong 'engine' (hw_engine) is selected for this exec_queue. Also, with this in mind, I believe this patch is entirely going to the wrong direction. We are stop validating the invalid_engine and trying to validate some other thing. If you need to validate the exec_queue_id you likely need a new test, instead of replacing this one like this. > } > > static void > @@ -329,8 +329,8 @@ igt_main > igt_subtest("invalid-ops") > invalid_ops(fd); > > - igt_subtest("invalid-engine") > - invalid_engine(fd); > + igt_subtest("invalid-exec_queue") > + invalid_exec_queue(fd); > > igt_subtest("exec_queue-reset-wait") > exec_queue_reset_wait(fd); > -- > 2.25.1 >