[RFC 0/2] Introduce a sysfs interface for lmem information

[Krzysztof Niemiec]

On 2025-05-21 at 09:06:43 GMT, Tvrtko Ursulin wrote:
> 
> On 20/05/2025 16:01, Joonas Lahtinen wrote:
> > (+ Tvrtko, Rodrigo and Jani)
> > 
> > Quoting Krzysztof Niemiec (2025-05-19 18:34:14)
> > > Hi,
> > > 
> > > This series introduces a way for applications to read local memory
> > > information via files in the sysfs. So far the only way to do this was
> > > via i915_query ioctl. This is slightly less handy than sysfs for
> > > external users. Additionally, the ioctl has a capability check which
> > > limits which users of a system might use it to get information.
> > > 
> > > The goals of this series are:
> > > 
> > >          1) Introduce a simpler interface to access lmem information,
> > >          2) Lift the CAP_PERFMON check on that information, OR provide
> > >             the administrator with a way to optionally lift it.
> > > 
> > > The first patch introduces the general mechanism without protections.
> > > This will effectively lift a capability check on obtaining the memory
> > > information. The second patch introduces that check back inside the
> > > _show() functions, but also adds a sysctl parameter allowing to override
> > > the checks, if an administrator so decides.
> > > 
> > > I'm sending this as RFC because I have a feeling that there's no
> > > consensus whether memory information exposed in the patch should be
> > > protected or not. Showing it to any user is strictly speaking an info
> > > leak, but the severity thereof might be considered not that high, so I'd
> > > rather leave it up to discussion first.
> > > 
> > > If we decide for lifting the check, the first patch is sufficient.
> > 
> > Nack on that.
> > 
> > CPU memory footprint and GPU memory footprint have a very different
> > nature. This was discussed to quite a length, please refer to mailing
> > list archives.
> > 
> > > If we
> > > decide against it, the second patch protects the information by default,
> > > but with a way to expose it as a conscious decision of the admin. I find
> > > it a decent compromise.
> > 
> > No need for the added complexity if we were to add a sysfs.
> > 
> > If a sysfs is added, it can be made root readable by default but system
> > admin is free to chown or chmod the file for more relaxed access. Back
> > in the original discussion time, this was omitted for lack of users.
> 
> Agreed, no need to complicate with redundant access controls in the kernel.
> 
> > Even now, userspace/sysadmin could already essentially use setuid helper
> > process that will only report the memory statistics.
> > 
> > So I'm not really fully convinced this is needed at all.
> > 
> > And if it is to be added for the convenience of usersppace, it should
> > probably then be considered to be a standard interface across DRM drivers
> > ala fdinfo or cgroups.
> 
> Cgroup visibility for device memory exists in principle although i915 hasn't
> been wired up to it.
> 
> For system memory (integrated GPUs) there is some work in progress around
> memcg accounting, although I am unsure if i915 would be automatically
> covered or not.
> 
> Also going a step back, from MangoHUDs point of view, I don't really see why
> total GPU memory is very interesting? I would have thought it is more
> interesting to know how much the monitored client is using, which is already
> available via fdinfo. Total memory sounds like something which it could
> leave to interpretation by the entity looking at the traces. (If the
> monitored client is running alone then total_free =~ total - client, and if
> it isn't running alone then it doesn't matter, no?)
> 

They use it to plot the VRAM usage so you have a rough idea of how much
of the total the client is using. [1]

[1] https://github.com/flightlessmango/MangoHud/blob/master/src/hud_elements.cpp#L1471-L1485

> Regards,
> 
> Tvrtko
> 
> > > This change has been requested in these parallel issues for i915 and Xe:
> > > 
> > > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14153
> > > https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/4861
> > > 
> > > Thanks
> > > Krzysztof
> > > 
> > > Krzysztof Niemiec (2):
> > >    drm/i915: Expose local memory information via sysfs
> > >    drm/i915: Add protections to sysfs local memory information
> > > 
> > >   drivers/gpu/drm/i915/i915_sysfs.c          |   6 +
> > >   drivers/gpu/drm/i915/intel_memory_region.c | 136 +++++++++++++++++++++
> > >   drivers/gpu/drm/i915/intel_memory_region.h |   3 +
> > >   3 files changed, 145 insertions(+)
> > > 
> > > -- 
> > > 2.45.2
> > > _
>