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 C08C8D591D4 for ; Tue, 19 Nov 2024 07:21:45 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 44FB710E02F; Tue, 19 Nov 2024 07:21:45 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="Yk+cQiiO"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id AFFD710E02F for ; Tue, 19 Nov 2024 07:21:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1732000904; x=1763536904; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=KZsAZ/E6Zqlw0D1wgXncEOwj/6oY/mPrvpNzwmz2pTY=; b=Yk+cQiiORNUb55WcPW7GtAEHXhi629xLRReEQrpgKVDHhVVuGPK4poE0 xxN+Uz5zTdxlYAL/Umyo9PH/eUWmCxgzTWlfcsa5PTuI6YzJ9jQyS8Bcd bZgkDoTRhDYp7auJtG+Uy2uE0rT4PXFmZwYKdtEgSusKYunGbfnuM5DoP HW68RJXySKhxweNrj5HAdF+eCCoCv9Kcyd9vQE6YXuGkKw0+Uni3gUjMe de9iYSOzUsaf68+gtL90/acahDBY6v9l7IKqiNLRuVVwFNNBm/Y0njXB0 zgpazVKPMtNVe+CCjNIt0pkgFM/RTlKjXZBadcmd9/nl+2Sau+Azc3ToX Q==; X-CSE-ConnectionGUID: mrAYBuycRq+GuRTMMMUrFQ== X-CSE-MsgGUID: SSf4AK/FS5+EjNTU98Jcrw== X-IronPort-AV: E=McAfee;i="6700,10204,11260"; a="31390624" X-IronPort-AV: E=Sophos;i="6.12,165,1728975600"; d="scan'208";a="31390624" Received: from orviesa005.jf.intel.com ([10.64.159.145]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Nov 2024 23:21:43 -0800 X-CSE-ConnectionGUID: xQWaTIvMSXOtqRQn6Bwmvg== X-CSE-MsgGUID: Zd7QGqDfT3mEYdQsxSqr/w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.12,165,1728975600"; d="scan'208";a="94401890" Received: from orsosgc001.jf.intel.com (HELO orsosgc001.intel.com) ([10.165.21.142]) by orviesa005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Nov 2024 23:21:42 -0800 Date: Mon, 18 Nov 2024 23:21:41 -0800 Message-ID: <8534jn7koq.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Pottumuttu, Sai Teja" Cc: igt-dev@lists.freedesktop.org Subject: Re: [PATCH i-g-t 2/2] tests/intel/xe_oa: Replace hardcoded OA buffer size assumptions In-Reply-To: <8096edaa-6b95-4436-9938-cf0e92821320@intel.com> References: <20241118063223.3974722-1-sai.teja.pottumuttu@intel.com> <20241118063223.3974722-3-sai.teja.pottumuttu@intel.com> <85v7wkwar3.wl-ashutosh.dixit@intel.com> <8096edaa-6b95-4436-9938-cf0e92821320@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-redhat-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII 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 Mon, 18 Nov 2024 23:01:50 -0800, Pottumuttu, Sai Teja wrote: > > On 19-11-2024 01:54, Dixit, Ashutosh wrote: > > On Sun, 17 Nov 2024 22:32:23 -0800, Sai Teja Pottumuttu wrote: > >> > >> A few xe_oa tests assumed that the default OA buffer size is 16MB > >> and had calculations around it. These would fail if the default > >> buffer size is changed on the kernel side. > >> > > > > I don't think this is correct. buf_size below refers to the size of the > > user's buffer, not the size of the kernel OA buffer. > > > > You also seem to be assuming that '65536 * report_size == oa_buffer_size' > > (65536 * 256 == 16 MB) but report size is not only 256, Xe2 default report > > size is 576 bytes and there are other report sizes too. > > > > Maybe ok to do what this patch is doing for test_buffer_fill and > > test_enable_disable, but we need to be careful about test_non_zero_reason > > since that test is very useful for testing OA buffer wrap-arounds (and some > > corner cases there). > > > > So, my assumption was that in i915 we were trying to do > > = (oa_buffer_size / report_size) * (report_size + header_size) > > which we hardcoded like > > = 65536 * (report_size + header_size) > > And this was done to get a buffer full of reports and allocate user buffer > a little bigger than kernel buffer as we had headers there. > > I think this translated into just 65536 * report_size in xe as we don't > have headers separately here. > > Am I correct with this assumption? Yes. > > 1. If yes, replacing it with oa_buffer_size is fine for atleast > test_buffer_fill and test_enable_disable right? As we were essentially > trying to allocate a user buffer which can take all the reports which would > fit in a kernel oa buffer. Am I missing something? > > If that's correct, as we are already at it, maybe we can keep the change > for these two tests? But even here, why would we link the kernel internal OA buffer size with the user buffer size? User buffer size can be independent of the OA buffer size. > 2. For test_non_zero_reason I think I get your point that with the current > sizing greater than 3 * oa_buffer_size, probably we could catch more > issues. So, we can keep it as it is probably. > > Thanks > - Sai Teja > > > I'd say let's drop this patch for now and revisit later if needed. > > > >> Replace such assumptions with the OA buffer size we get from > >> DRM_XE_OBSERVATION_IOCTL_INFO ioctl > >> > >> Signed-off-by: Sai Teja Pottumuttu > >> --- > >> tests/intel/xe_oa.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c > >> index 4e4d0fabc..3b65212ae 100644 > >> --- a/tests/intel/xe_oa.c > >> +++ b/tests/intel/xe_oa.c > >> @@ -2389,7 +2389,7 @@ test_buffer_fill(const struct drm_xe_engine_class_instance *hwe) > >> .properties_ptr = to_user_pointer(properties), > >> }; > >> size_t report_size = get_oa_format(fmt).size; > >> - int buf_size = 65536 * report_size; > >> + int buf_size = oa_buffer_size; > >> uint8_t *buf = malloc(buf_size); > >> int len; > >> size_t oa_buf_size = oa_buffer_size; > >> @@ -2533,7 +2533,7 @@ test_non_zero_reason(const struct drm_xe_engine_class_instance *hwe) > >> .num_properties = ARRAY_SIZE(properties) / 2, > >> .properties_ptr = to_user_pointer(properties), > >> }; > >> - uint32_t buf_size = 3 * 65536 * report_size; > >> + uint32_t buf_size = 3 * oa_buffer_size; > >> uint8_t *buf = malloc(buf_size); > >> uint32_t total_len = 0; > >> const uint32_t *last_report; > >> @@ -2615,7 +2615,7 @@ test_enable_disable(const struct drm_xe_engine_class_instance *hwe) > >> .properties_ptr = to_user_pointer(properties), > >> }; > >> size_t report_size = get_oa_format(fmt).size; > >> - int buf_size = 65536 * report_size; > >> + int buf_size = oa_buffer_size; > >> uint8_t *buf = malloc(buf_size); > >> size_t oa_buf_size = oa_buffer_size; > >> int n_full_oa_reports = oa_buf_size / report_size; > >> -- > >> 2.34.1 > >> > > > > Thanks. > > -- > > Ashutosh