linux-kernel - Re: [Intel-gfx] [PATCH v6 08/17] drm/ttm: use gem vma

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190814093524.GA31345@ulmo>
Date:   Wed, 14 Aug 2019 11:35:24 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Gerd Hoffmann <kraxel@...hat.com>
Cc:     Ben Skeggs <bskeggs@...hat.com>, dri-devel@...ts.freedesktop.org,
        nouveau@...ts.freedesktop.org, amd-gfx@...ts.freedesktop.org,
        linux-graphics-maintainer@...are.com,
        intel-gfx@...ts.freedesktop.org, spice-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [Intel-gfx] [PATCH v6 08/17] drm/ttm: use gem vma_node

On Wed, Aug 14, 2019 at 07:58:27AM +0200, Gerd Hoffmann wrote:
> > Hi Gerd,
> > 
> > I've been seeing a regression on Nouveau with recent linux-next releases
> > and git bisect points at this commit as the first bad one. If I revert
> > it (there's a tiny conflict with a patch that was merged subsequently),
> > things are back to normal.
> > 
> > I think the reason for this issue is that Nouveau doesn't use GEM
> > objects for all buffer objects,
> 
> That shouldn't be a problem ...
> 
> > and even when it uses GEM objects, the
> > code will not initialize the GEM object until after the buffer objects
> > and the backing TTM objects have been created.
> 
> ... but the initialization order is.
> 
> ttm_bo_uses_embedded_gem_object() assumes gem gets initialized first.
> 
> drm_gem_object_init() init calling drm_vma_node_reset() again is
> probably the root cause for the breakage.
> 
> > I tried to fix that by making sure drm_gem_object_init() gets called by
> > Nouveau before ttm_bo_init(), but the changes are fairly involved and I
> > was unable to get the GEM reference counting right. I can look into the
> > proper fix some more, but it might be worth reverting this patch for
> > now to get Nouveau working again.
> 
> Changing the order doesn't look hard.  Patch attached (untested, have no
> test hardware).  But maybe I missed some detail ...
> 
> The other patch attached works around the issue with a flag, to avoid
> drm_vma_node_reset() being called twice.

I came up with something very similar by splitting up nouveau_bo_new()
into allocation and initialization steps, so that when necessary the GEM
object can be initialized in between. I think that's slightly more
flexible and easier to understand than a boolean flag.

Thierry

View attachment "0001-drm-nouveau-Initialize-GEM-object-before-TTM-object.patch" of type "text/plain" (8796 bytes)

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)