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 4CCB4CD4F54 for ; Fri, 29 May 2026 23:03:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 501A71125E2; Fri, 29 May 2026 23:03:16 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="DMrDpkuo"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id DD3881125DC for ; Fri, 29 May 2026 23:03:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1780095795; x=1811631795; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=sCytmJ1XJPVF2SGjYpTsBZq/1u4XkDq4fAt4tf63Ouo=; b=DMrDpkuol/tTEFwOT/GA9dU3mkaVZmtAeMBmiMRC0cd5B67WZSrOTd0Q eS+gdtUwREU7wQrHD9A/QlIffsHCfzzaAqk3+5xBYwzZMGx3zJnPJ1983 Y/dXYxRG9hIn3Qy1DqQwYA5241ppvNoT1/L3b+PL6OnLJNZR+bZ4uHYS0 GOQ+4HA+FMx9OOUjzRZd09ODvtW2XWkwPd9wJ8b7KZbZ05MpIH2CAe6N6 s2TAfDTVqEvrBzeKiVSx+RfopNxOSMGo7MOziP5pGi4ikrpzXSK6Jd+nT IdxA7f4LREiYdCCnhAbSoiC8zQnXmbLBPrWrnJDfArPplWO+l40NNM8VV A==; X-CSE-ConnectionGUID: PTfMnD2mR866S/ZzE1PIWA== X-CSE-MsgGUID: Vga3xkUeRi671f96029PpA== X-IronPort-AV: E=McAfee;i="6800,10657,11801"; a="103623560" X-IronPort-AV: E=Sophos;i="6.24,176,1774335600"; d="scan'208";a="103623560" Received: from orviesa003.jf.intel.com ([10.64.159.143]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 May 2026 16:03:15 -0700 X-CSE-ConnectionGUID: wDjkikomTmWj3dyo++eB+g== X-CSE-MsgGUID: A7l1TTxzS8OIg/Ju0t9gdA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.24,176,1774335600"; d="scan'208";a="246984377" Received: from sgreig-mobl1.amr.corp.intel.com (HELO adixit-MOBL3.intel.com) ([10.125.35.95]) by ORVIESA003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 May 2026 16:03:14 -0700 Date: Fri, 29 May 2026 16:03:13 -0700 Message-ID: <874ijpn7im.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa Cc: Subject: Re: [PATCH 7/9] drm/xe/rtp: (De-)whitelist OA registers for all hwe's for a gt In-Reply-To: References: <20260518234716.1540123-1-ashutosh.dixit@intel.com> <20260518234716.1540123-8-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 Wed, 27 May 2026 14:49:12 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > On Mon, May 18, 2026 at 04:47:14PM -0700, Ashutosh Dixit wrote: > > Whitelist or de-whitelist OA registers for all hwe's on the gt on which the > > OA stream is opened. This simplifies the case where an oa unit has 0 > > attached hwe's (but which monitors OA events on the associated GT). > > > > Signed-off-by: Ashutosh Dixit > > --- > > drivers/gpu/drm/xe/xe_reg_whitelist.c | 32 ++++++++++++++++++++++++++- > > drivers/gpu/drm/xe/xe_reg_whitelist.h | 4 ++++ > > 2 files changed, 35 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_reg_whitelist.c b/drivers/gpu/drm/xe/xe_reg_whitelist.c > > index 0d2cf3d964a51..50b34c5008df7 100644 > > --- a/drivers/gpu/drm/xe/xe_reg_whitelist.c > > +++ b/drivers/gpu/drm/xe/xe_reg_whitelist.c > > @@ -229,7 +229,7 @@ void xe_reg_whitelist_process_engine(struct xe_hw_engine *hwe) > > whitelist_apply_to_hwe(hwe, &hwe->oa_whitelist, &hwe->reg_sr, nonpriv_slots); > > } > > > > -__maybe_unused static void __whitelist_oa_regs(struct xe_hw_engine *hwe, bool whitelist) > > +static void __whitelist_oa_regs(struct xe_hw_engine *hwe, bool whitelist) > > { > > struct xe_reg_sr_entry *entry; > > unsigned long reg; > > @@ -244,6 +244,36 @@ __maybe_unused static void __whitelist_oa_regs(struct xe_hw_engine *hwe, bool wh > > xe_reg_sr_apply_mmio(&hwe->oa_sr, hwe->gt); > > } > > > > +/** > > + * xe_reg_whitelist_oa_regs - whitelist oa registers for gt > > + * @gt: gt to whitelist oa registers for > > + * > > + * Whitelist OA registers by resetting RING_FORCE_TO_NONPRIV_DENY > > + */ > > +void xe_reg_whitelist_oa_regs(struct xe_gt *gt) > > +{ > > + struct xe_hw_engine *hwe; > > + enum xe_hw_engine_id id; > > + > > + for_each_hw_engine(hwe, gt, id) > > + __whitelist_oa_regs(hwe, true); > > I think we should only apply the whitelist to the hwe that will use this > specific OA unit. That will help do away with the ref counting in patch 9 > here. No, if we do this we will need to introduce per hwe refcounts (instead of per gt refcounts, as done in Patch 9). Each OA unit has a set of attached hwe's. Then there are OA units such as oam-sag and mertoa, which don't have any attached hwe's. For these we allow mmio triggers to be submitted on any hwe's attached to that gt. Also some hwe's like copy engines are common between OA units. Since multiple OA streams might be open simultaneously, because of these reasons, we'd have to introduce per hwe refcounts. Also, if the concern is that we are opening up all hwe's on a gt, rather than hwe's attached to the OA unit on which an OA stream is opened, I do not consider this a serious issue, because after all registers are being whitlisted by an explicit permission from root. The root, e.g. is free to load up a custom module which can whitelist whatever they want. So, if we want to implement whitelisting only hwe's attached to the an OA unit (including OA units which don't have any hwe's attached, so would include all hwe's on a gt), this is substantial increase in code complexity, that will need a separate series of patches, over and above the current series. So if we do this, that's a separate set of patches. Since the current series is already a big improvement in closing down unconditional OA register whitelisting, I'd say let's get this reviewed and merged first and then we debate if we at all need to restrict the whitelisting further and then do that if we need to, but I really don't think it is needed. Thanks. -- Ashutosh > > +} > > + > > +/** > > + * xe_reg_dewhitelist_oa_regs - dewhitelist oa registers for gt > > + * @gt: gt to dewhitelist oa registers for > > + * > > + * Dewhitelist OA registers by setting RING_FORCE_TO_NONPRIV_DENY > > + */ > > +void xe_reg_dewhitelist_oa_regs(struct xe_gt *gt) > > +{ > > + struct xe_hw_engine *hwe; > > + enum xe_hw_engine_id id; > > + > > + for_each_hw_engine(hwe, gt, id) > > + __whitelist_oa_regs(hwe, false); > > +} > > + > > /** > > * xe_reg_whitelist_print_entry - print one whitelist entry > > * @p: DRM printer > > diff --git a/drivers/gpu/drm/xe/xe_reg_whitelist.h b/drivers/gpu/drm/xe/xe_reg_whitelist.h > > index 3b64b42fe96e9..e1eb1b7d5480b 100644 > > --- a/drivers/gpu/drm/xe/xe_reg_whitelist.h > > +++ b/drivers/gpu/drm/xe/xe_reg_whitelist.h > > @@ -9,12 +9,16 @@ > > #include > > > > struct drm_printer; > > +struct xe_gt; > > struct xe_hw_engine; > > struct xe_reg_sr; > > struct xe_reg_sr_entry; > > > > void xe_reg_whitelist_process_engine(struct xe_hw_engine *hwe); > > > > +void xe_reg_whitelist_oa_regs(struct xe_gt *gt); > > +void xe_reg_dewhitelist_oa_regs(struct xe_gt *gt); > > + > > void xe_reg_whitelist_print_entry(struct drm_printer *p, unsigned int indent, > > u32 reg, struct xe_reg_sr_entry *entry); > > > > -- > > 2.54.0 > >