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 0B1B3C43458 for ; Fri, 26 Jun 2026 22:39:37 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8A07F10E3DB; Fri, 26 Jun 2026 22:39:37 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="cZlQVHS8"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.17]) by gabe.freedesktop.org (Postfix) with ESMTPS id CFAD010E3DB for ; Fri, 26 Jun 2026 22:39:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1782513576; x=1814049576; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=Jld3+tp7CtlrzFG9COc3KF3a3hbtlfVSts+QxJqAWLY=; b=cZlQVHS81l18c9JDLmkcgoWXSBPcg+AHU6iYe4PyWnAyUooodSzyXA9I wjYkYs/5wvcCp1LF2pFjoMbjLJ+qSjppOCPilu0aojImpEhZL+fJmU7s4 RVZng8kgeURR2swyINdSj2Mpe4D8uDNpNxgK64hJzE8a/ZxT97iancX48 BhbqmJwj2OAUkN6N3ybjAl9f+ZntWckVeum+GSp4a2zf+gtkOXS9+aPGG 1ki1UNk70bxZmBApzruZaBdFD/GJitJy/WbJvbOzqtpdZJtfY8OLbW/aJ P1+hMhWEk2arlWK+LOpXdErNz1zwQrM4Ms/LhWci6+EUk+Ig3t+qoD08C A==; X-CSE-ConnectionGUID: a2xU6E7dR+OgU8Iid/c40Q== X-CSE-MsgGUID: M6GG6BuwT6yAUC69YBCfEQ== X-IronPort-AV: E=McAfee;i="6800,10657,11829"; a="83188018" X-IronPort-AV: E=Sophos;i="6.24,227,1774335600"; d="scan'208";a="83188018" Received: from fmviesa003.fm.intel.com ([10.60.135.143]) by fmvoesa111.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Jun 2026 15:39:35 -0700 X-CSE-ConnectionGUID: 9OZCBMmFSceKIzYDOpx+Yw== X-CSE-MsgGUID: 2j8euhMNSoGEDvTRn+6Jig== X-ExtLoop1: 1 Received: from huyngu6x-mobl1.amr.corp.intel.com (HELO adixit-MOBL3.intel.com) ([10.125.34.9]) by fmviesa003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Jun 2026 15:39:35 -0700 Date: Fri, 26 Jun 2026 15:39:34 -0700 Message-ID: <875x35orix.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa Cc: Subject: Re: [PATCH v2 3/4] drm/xe/oa: Provide OA status through status ioctl for mmap interface In-Reply-To: References: <20260617015422.226177-1-ashutosh.dixit@intel.com> <20260617015422.226177-4-ashutosh.dixit@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/30.2 (x86_64-pc-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: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, 25 Jun 2026 16:16:33 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > On Tue, Jun 16, 2026 at 06:54:21PM -0700, Ashutosh Dixit wrote: > > When only the OA mmap interface is in use, OA status can be provided > > through the status ioctl. This enables us to stop whitelisting the OA > > status register, thereby saving one nonpriv slot per OA unit. > > > > Signed-off-by: Ashutosh Dixit > > --- > > drivers/gpu/drm/xe/xe_oa.c | 7 +++++++ > > drivers/gpu/drm/xe/xe_oa_types.h | 3 +++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c > > index 9fbd21b0ef974..985be10855c33 100644 > > --- a/drivers/gpu/drm/xe/xe_oa.c > > +++ b/drivers/gpu/drm/xe/xe_oa.c > > @@ -544,6 +544,7 @@ static int __xe_oa_read(struct xe_oa_stream *stream, char __user *buf, > > /* Only clear our bits to avoid side-effects */ > > stream->oa_status = xe_mmio_rmw32(&stream->gt->mmio, __oa_regs(stream)->oa_status, > > OASTATUS_RELEVANT_BITS, 0); > > + stream->oa_status_read = true; > > /* > > * Signal to userspace that there is non-zero OA status to read via > > * @DRM_XE_OBSERVATION_IOCTL_STATUS observation stream fd ioctl > > @@ -1603,6 +1604,12 @@ static long xe_oa_status_locked(struct xe_oa_stream *stream, unsigned long arg) > > struct drm_xe_oa_stream_status status = {}; > > void __user *uaddr = (void __user *)arg; > > > > + /* Provide oa_status through the status ioctl, when only mmap() interface is used */ > > + if (!stream->oa_status_read) > > + stream->oa_status = xe_mmio_rmw32(&stream->gt->mmio, __oa_regs(stream)->oa_status, > > + OASTATUS_RELEVANT_BITS, 0); > > + stream->oa_status_read = false; > > + > > This is asymmetric especially since we cannot control a user calling mmap > and read from the same application. I would suggest that we avoid clearing > the register status bits in _xe_oa_read. read() will continue to return > EIO until the status ioctl is called. In the status ioctl, we always read > the latest value from the HW REG, return that and then clear the register > bits. This is the text in xe_drm.h: " * %DRM_XE_OBSERVATION_IOCTL_STATUS observation stream fd ioctl. Userspace can * call the ioctl to query stream status in response to EIO errno from * observation fd read(). " So, at least in my mind, calling the status ioctl was always optional (indicated by "Userspace *can* call") allowing userspace to ignore the EIO return and still be able to continue. So if we do what you suggest above, even our own IGT's will need to be fixed, because they can hang, if EIO is returned continuously. There is no perfect solution to this problem and that is why I came up with the scheme in this patch, which to me seems to be good enough and works for all common use cases. Thanks. -- Ashutosh > > /* Map from register to uapi bits */ > > if (stream->oa_status & OASTATUS_REPORT_LOST) > > status.oa_status |= DRM_XE_OASTATUS_REPORT_LOST; > > diff --git a/drivers/gpu/drm/xe/xe_oa_types.h b/drivers/gpu/drm/xe/xe_oa_types.h > > index 3d9ec8490899c..a84456540a13c 100644 > > --- a/drivers/gpu/drm/xe/xe_oa_types.h > > +++ b/drivers/gpu/drm/xe/xe_oa_types.h > > @@ -244,6 +244,9 @@ struct xe_oa_stream { > > /** @oa_status: temporary storage for oa_status register value */ > > u32 oa_status; > > > > + /** @oa_status_read: true when read() interface is used to read oa_status */ > > + bool oa_status_read; > > + > > /** @no_preempt: Whether preemption and timeslicing is disabled for stream exec_q */ > > u32 no_preempt; > > > > -- > > 2.54.0 > >