linux-kernel - Re: [PATCH v4 2/2] Documentation/gpu: Add fdinfo meanings of drm-*-internal memory tags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <knnhq3aehfy7r5gy7l5z4sluzyghpz22mu66lei6ysnhvzxz2c@sajpyqyu64qq>
Date: Thu, 19 Dec 2024 01:09:42 +0800
From: Adrián Larumbe <adrian.larumbe@...labora.com>
To: Liviu Dudau <liviu.dudau@....com>
Cc: Boris Brezillon <boris.brezillon@...labora.com>, 
	Steven Price <steven.price@....com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, kernel@...labora.com, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] Documentation/gpu: Add fdinfo meanings of
 drm-*-internal memory tags

On 16.12.2024 14:25, Liviu Dudau wrote:
> On Wed, Dec 11, 2024 at 04:34:32PM +0000, Adrián Larumbe wrote:
> > A previous commit enabled display of driver-internal kernel BO sizes
> > through the device file's fdinfo interface.
> > 
> > Expand the description of the relevant driver-specific key:value pairs
> > with the definitions of the new drm-*-internal ones.
> > 
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@...labora.com>
> > ---
> >  Documentation/gpu/panthor.rst | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/gpu/panthor.rst b/Documentation/gpu/panthor.rst
> > index 3f8979fa2b86..c6d8236e3665 100644
> > --- a/Documentation/gpu/panthor.rst
> > +++ b/Documentation/gpu/panthor.rst
> > @@ -26,6 +26,10 @@ the currently possible format options:
> >       drm-cycles-panthor:     94439687187
> >       drm-maxfreq-panthor:    1000000000 Hz
> >       drm-curfreq-panthor:    1000000000 Hz
> > +     drm-total-internal:     10396 KiB
> > +     drm-shared-internal:    0
> > +     drm-active-internal:    10396 KiB
> > +     drm-resident-internal:  10396 KiB
> >       drm-total-memory:       16480 KiB
> >       drm-shared-memory:      0
> >       drm-active-memory:      16200 KiB
> > @@ -44,3 +48,13 @@ driver by writing into the appropriate sysfs node::
> >  
> >  Where `N` is a bit mask where cycle and timestamp sampling are respectively
> >  enabled by the first and second bits.
> > +
> > +Possible `drm-*-internal` key names are: `total`, `active` and `resident`.
> 
> I think Mihail's comment stands. There is no harm in being thorough, so either
> we list `shared` as a possible key name, or we remove it from the example and
> re-introduce it later with a patch.

I'll mention it here too, even though there is no way I can prevent 'drm-shared-internal'
from being displayed because drm_print_memory_stats() is inflexible about this.

> > +These values convey the sizes of the internal driver-owned shmem BO's that
> > +aren't exposed to user-space through a DRM handle, like queue ring buffers,
> > +sync object arrays and heap chunks. Because they are all allocated and pinned
> > +at creation time, `drm-resident-internal` and `drm-total-internal` should always
> > +be equal. `drm-active-internal` shows the size of kernel BO's associated with
> > +VM's and groups currently being scheduled for execution by the GPU.
> > +`drm-shared-memory` is unused at present, but in the future it might stand for
> > +the size of the Firmware regions, since they do not belong to an open file context.
> 
> But there are firmware regions that are per context, like the save/restore buffers.
> Also, I think we are creating confusion there between drm-shared-memory and
> drm-shared-internal. I'm fine with having a note here regarding `drm-shared-memory`
> as long as `drm-shared-internal` also gets a note to explain the difference with its
> cousin.

This was a typo on my part, should've written 'drm-shared-internal' instead because the
definition of drm-shared-memory is done in the documentation for the DRM fdinfo core.
In the next revision, I'll also mention that the potential future meaning of drm-shared-internal
could be the size of the executable FW regions to dispel any ambiguities.

> Best regards,
> Liviu
> 
> > -- 
> > 2.47.0
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯

Adrian Larumbe