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

[Krzysztof Niemiec]

Hi,

On 2025-05-20 at 19:46:48 GMT, Andi Shyti wrote:
> 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.
> 

I'm sorry if I wasn't clear enough in the cover letter; client is
MangoHUD[1]. It's a popular overlay for benchmarking games. They reached
out to us because they have no convenient way to read total memory
information.

[1] https://github.com/flightlessmango/MangoHud

Thanks
Krzysztof

> 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
> > > _