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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ab8f30452540171447118d64931e66da96b6e85e.camel@linux.intel.com>
Date:   Tue, 31 Oct 2023 17:45:56 +0100
From:   Thomas Hellström 
        <thomas.hellstrom@...ux.intel.com>
To:     Danilo Krummrich <dakr@...hat.com>, airlied@...il.com,
        daniel@...ll.ch, matthew.brost@...el.com, sarah.walker@...tec.com,
        donald.robson@...tec.com, boris.brezillon@...labora.com,
        christian.koenig@....com, faith@...strand.net
Cc:     dri-devel@...ts.freedesktop.org, nouveau@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH drm-misc-next v7 4/7] drm/gpuvm: add an abstraction for
 a VM / BO combination

On Tue, 2023-10-31 at 17:39 +0100, Danilo Krummrich wrote:
> On 10/31/23 12:25, Thomas Hellström wrote:
> > On Mon, 2023-10-23 at 22:16 +0200, Danilo Krummrich wrote:
> > > Add an abstraction layer between the drm_gpuva mappings of a
> > > particular
> > > drm_gem_object and this GEM object itself. The abstraction
> > > represents
> > > a
> > > combination of a drm_gem_object and drm_gpuvm. The drm_gem_object
> > > holds
> > > a list of drm_gpuvm_bo structures (the structure representing
> > > this
> > > abstraction), while each drm_gpuvm_bo contains list of mappings
> > > of
> > > this
> > > GEM object.
> > > 
> > > This has multiple advantages:
> > > 
> > > 1) We can use the drm_gpuvm_bo structure to attach it to various
> > > lists
> > >     of the drm_gpuvm. This is useful for tracking external and
> > > evicted
> > >     objects per VM, which is introduced in subsequent patches.
> > > 
> > > 2) Finding mappings of a certain drm_gem_object mapped in a
> > > certain
> > >     drm_gpuvm becomes much cheaper.
> > > 
> > > 3) Drivers can derive and extend the structure to easily
> > > represent
> > >     driver specific states of a BO for a certain GPUVM.
> > > 
> > > The idea of this abstraction was taken from amdgpu, hence the
> > > credit
> > > for
> > > this idea goes to the developers of amdgpu.
> > > 
> > > Cc: Christian König <christian.koenig@....com>
> > > Signed-off-by: Danilo Krummrich <dakr@...hat.com>
> > > ---
> > >   drivers/gpu/drm/drm_gpuvm.c            | 335
> > > +++++++++++++++++++++--
> > > --
> > >   drivers/gpu/drm/nouveau/nouveau_uvmm.c |  64 +++--
> > >   include/drm/drm_gem.h                  |  32 +--
> > >   include/drm/drm_gpuvm.h                | 188 +++++++++++++-
> > >   4 files changed, 533 insertions(+), 86 deletions(-)
> > 
> > That checkpatch.pl error still remains as well.
> 
> I guess you refer to:
> 
> ERROR: do not use assignment in if condition
> #633: FILE: drivers/gpu/drm/nouveau/nouveau_uvmm.c:1165:
> +                       if (!(op->gem.obj = obj))
> 
> This was an intentional decision, since in this specific case it
> seems to
> be more readable than the alternatives.
> 
> However, if we consider this to be a hard rule, which we never ever
> break,
> I'm fine changing it too.

With the errors, sooner or later they are going to start generate
patches to "fix" them. In this particular case also Xe CI is
complaining and abort building when I submit the Xe adaptation, so it'd
be good to be checkpatch.pl conformant IMHO.

Thanks,
Thomas




> 
> > 
> > Thanks,
> > Thomas
> > 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ