Re: [PATCH v2 1/4] drm/xe/oa: Rename last argument of WHITELIST_OA_MMIO_TRG

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Fri, 26 Jun 2026 09:39:37 -0700, Matt Roper wrote:
>

Hi Matt,

> On Tue, Jun 16, 2026 at 06:54:19PM -0700, Ashutosh Dixit wrote:
> > OA head pointer registers are not used by UMD's and do not need to be
> > whitelisted, the last argument of WHITELIST_OA_MMIO_TRG is actually used
> > for whitelisting tail pointer and OA buffer registers. Rename the argument
> > to tail_buf to reflect this. OA head pointer is sometimes provided to the
> > WHITELIST_OA_MMIO_TRG to have the correct register offset alignment (16)
> > for RING_FORCE_TO_NONPRIV_RANGE_4.
>
> We should only be using that in cases where we definitely have four
> consecutive registers that _all_ need the be accessed by userspace and
> that are documented as okay to whitelist in the specs.  If userspace only
> has a need for two consecutive registers, there's no reason to be using
> RANGE_4.  Granting userspace access to two extra unnecessary registers is
> a DRM uapi violation (and also a potential security risk depending on
> what the extra two registers wind up being).

We recently merged

https://patchwork.freedesktop.org/series/166809/

So OA registers are basically only whitelisted now with "explicit
permission from root" (they are not whitelisted by default and also we are
preparing series to fix this up in previous stable kernel versions
too). Our position is that if root has given explicit permission, "all bets
are off" (I will write a patch documenting the sort of things which can
happen).

So imo we should treat OA register whitelisting differently from other
non-OA register whitelisting. Further, we asked our security guys
specifically about the use of RANGE_4 for OA and they have okay'd it.

> I think the proper fix here is to get rid of RANGE_4.

There are few things in flight here (explained below), but the reason for
retaining RANGE_4 was discussed here:

https://lore.kernel.org/intel-xe/87ecin7259.wl-ashutosh.dixit@intel.com/

So if we drop RANGE_4, we will currently use up 12 nonpriv slots for each
media engine (leaving no free slots for other workarounds etc.). Of course,
we can fix and merge the patch to increase the available nonpriv slots from
12 to 20. In patches 3 and 4 of this series, we are also trying to stop
whitelisting OASTATUS, so it that goes through (there's some discussion on
Patch 3 currently), that will also free up 1 nonpriv slot (3 in total for
each media engine). So if we do any of this, maybe we will still be able to
drop RANGE_4.

But the question would still be should we still save 3 nonpriv slots or not
(for media engines and 1 nonpriv slot for rcs/ccs). But anyway I think
we'll have to do one of the things mentioned above before we can drop
RANGE_4.

Thanks.
--
Ashutosh

> > Fixes: ed455775c5a6 ("drm/xe/rtp: Refactor OAG MMIO trigger register whitelisting")
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_reg_whitelist.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_reg_whitelist.c b/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > index 2e84b1c49f374..a17ebacc1455b 100644
> > --- a/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > +++ b/drivers/gpu/drm/xe/xe_reg_whitelist.c
> > @@ -104,10 +104,10 @@ static const struct xe_rtp_table_sr register_whitelist = XE_RTP_TABLE_SR(
> >				   RING_FORCE_TO_NONPRIV_ACCESS_RW))
> >	},
> >
> > -#define WHITELIST_OA_MMIO_TRG(trg, status, head) \
> > +#define WHITELIST_OA_MMIO_TRG(trg, status, tail_buf) \
> >	WHITELIST(trg, RING_FORCE_TO_NONPRIV_ACCESS_RW), \
> >	WHITELIST(status, RING_FORCE_TO_NONPRIV_ACCESS_RD), \
> > -	WHITELIST(head, RING_FORCE_TO_NONPRIV_ACCESS_RD | RING_FORCE_TO_NONPRIV_RANGE_4)
> > +	WHITELIST(tail_buf, RING_FORCE_TO_NONPRIV_ACCESS_RD | RING_FORCE_TO_NONPRIV_RANGE_4)
> >
> >  #define WHITELIST_OAG_MMIO_TRG \
> >	WHITELIST_OA_MMIO_TRG(OAG_MMIOTRIGGER, OAG_OASTATUS, OAG_OAHEADPTR)
> > --
> > 2.54.0
> >