Re: [PATCH 00/17] Add OA functionality to Xe

["Souza, Jose" <jose.souza@intel.com>]

On Tue, 2024-05-21 at 09:43 -0700, Dixit, Ashutosh wrote:
> On Tue, 21 May 2024 09:29:51 -0700, Souza, Jose wrote:
> > 
> > On Tue, 2024-05-21 at 09:10 -0700, Dixit, Ashutosh wrote:
> > > On Tue, 21 May 2024 07:47:58 -0700, Souza, Jose wrote:
> > > 
> > > Hi Jose,
> > > 
> > > > > Other ask, can you remove this 'Failed to remove unknown OA config'
> > > > > debug message from xe_oa_remove_config_ioctl()?
> > > > 
> > > > Missed 'Insufficient privileges to remove xe OA config', that need to be
> > > > removed too from xe_oa_remove_config_ioctl().
> > > > 
> > > > > Mesa will be using DRM_XE_PERF_OP_REMOVE_CONFIG with config id set to
> > > > > UINT64_MAX to detect if Xe KMD supports OA counters and if application
> > > > > has enough permissions to use it.  So it causes dmesg to be flooded
> > > > > with 'xe 0000:00:02.0: [drm:xe_oa_remove_config_ioctl [xe]] Failed to
> > > > > remove unknown OA config' messages when running tests suites.
> > > > > 
> > > > > Or do you have other suggestion of uAPI that I can use.
> > > 
> > > OK, so you are relying on ENODEV and EACCES errno's from
> > > DRM_XE_PERF_OP_REMOVE_CONFIG to find out (a) if OA is present and (b) if
> > > you need to be root (actually CAP_PERFMON or CAP_SYS_ADMIN).
> > 
> > yep
> > 
> > > 
> > > This logic in Xe should be close to what we have in i915? What does Mesa do
> > > for i915, or what doesn't work in Xe?
> > > 
> > > Here are some pointers:
> > > 
> > > * You can execute DRM_XE_DEVICE_QUERY_OA_UNITS to see if OA is present
> > > 
> > > * Add/remove OA configs and using the global OAG buffer (time based
> > >   sampling or DRM_XE_OA_PROPERTY_SAMPLE_OA set) are priviliged operations
> > >   (need root). Operations which only need OAR/OAC (OA queries, without
> > >   DRM_XE_OA_PROPERTY_SAMPLE_OA) can be executed by non-root.
> > > 
> > > * If "/proc/sys/dev/xe/perf_stream_paranoid" is 0, all operations can be
> > >   executed by non-root users. Otherwise, as I described in the previous
> > >   point.
> > 
> > It is possible that process not started by root has CAP_PERFMON:
> 
> Yes, correct.
> 
> Above I am using "root" loosely as "CAP_PERFMON or CAP_SYS_ADMIN".
> 
> So if root sets 'perf_stream_paranoid' to 0, normal users can do OA
> priviliged operations (as described above).
> 
> If root sets 'perf_stream_paranoid' to 1, only CAP_PERFMON or CAP_SYS_ADMIN
> users can do OA priviliged operations.

oh okay so perf_stream_paranoid has a good usage but it do not covers CAP_PERFMON, see below.

> 
> > "Unprivileged processes with enabled CAP_PERFMON capability are treated
> > as privileged processes with respect to perf_events performance
> > monitoring and observability operations,..."
> > 
> > And from what I understood only root can write to perf_stream_paranoid,
> > so I don't see a point in having this file...
> 
> So I don't know how this statement follows?
> 
> root or superuser is the one which gives the permission to non-CAP_PERFMON
> and non-CAP_SYS_ADMIN users to be able to do priviliged OA operations.

so if I'm running a process with CAP_PERFMON and read perf_stream_paranoid and it returns 0 there is no way for UMD to know that process is allowed to
use Xe KMD OA feature without do a uAPI call that checks for CAP_PERFMON.

That is why I think is better just do a single DRM_XE_PERF_OP_REMOVE_CONFIG to detect if feature is present and if process is allowed to use it. But
it generates a bunch of debug messages.

> 
> > > So basically I think you just need to check for the perf_stream_paranoid
> > > file above. It will tell you both (a) if OA is present (because we are
> > > going to merge the code which creates this file together with OA) and (b)
> > > if you need to be root for particular operations.
> > > 
> > > Thanks.
> > > --
> > > Ashutosh
> >