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

[Andi Shyti]

Hi,

On Tue, May 20, 2025 at 06:01:12PM +0300, Joonas Lahtinen wrote:
> Quoting Krzysztof Niemiec (2025-05-19 18:34:14)
> > 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.
> 
> 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.

yeah! What is the real use case? Who is the userspace client?

There are already ways to read out the GPU memory footprint so
that we need to know whether we need for another uAPI.

Andi

> 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.
> 
> Regards, Joonas
> 
> > 
> > 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
> > _