[PATCH v2 00/20] Unified VMA Offset Manager v2 (+Render Node RFC)

2013-07-11 Thread Martin Peres
On 11/07/2013 13:21, David Herrmann wrote:
> Hi
>
> On Thu, Jul 11, 2013 at 1:12 PM, Martin Peres  
> wrote:
>> On 07/07/2013 19:17, David Herrmann wrote:
>>> Hi
>>>
>>> This is v2 of the unified VMA offset manager series. The first draft is
>>> available at LWN [1]. This series replaces the VMA offset managers in GEM
>>> and
>>> TTM with a unified implementation.
>>>
>>> The first 4 patches contain the new VMA offset manager and are the only
>>> patches
>>> that I propose for mainline inclusion.
>>> Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar
>>> patches in i915-next and I will rebase them once it is merged. Ignore
>>> them if you're not interested.
>>> Patches 9-19 implement mmap access control. Comments are welcome! They are
>>> intended for mainline inclusion, too, but probably require some more
>>> review
>>> rounds. I'd really appreciate if driver authors could comment on the
>>> implementation.
>>> Patch 20 shows the main motivation behind this whole series: render nodes.
>>> It is
>>> marked RFC and I will resend once the VMA manager is merged upstream. Feel
>>> free
>>> to test it.
>>>
>>> One more note regarding DRM-Master: Render-clients are independent of
>>> DRM-Master
>>> (see the DocBook documentation). It really doesn't make sense to
>>> create/bind a
>>> DRM-Master object to render-clients. However, a lot of core DRM code
>>> depends on
>>>->master != NULL. Drivers need to take care not to call into those core
>>> modules
>>> if they implement DRIVER_RENDER.
>>> I plan on removing multiple-master-support in the next series. So there
>>> will be
>>> only one global master and each open-file is bound to it. This will make
>>> it very
>>> easy to phase out "drm_master". The planned "modeset" nodes provide a nice
>>> replacement. If anyone knows a **currently used** use-case for
>>> multiple-masters,
>>> please tell me. I couldn't find one that _actually works_.
>>> (SetMaster+DropMaster
>>> will obviously stay functional even with drm_master removed).
>>>
>>>
>>> (series available at: https://github.com/dvdhrm/linux/tree/rnodes)
>>>
>>> Comments welcome!
>>> Cheers
>>> David
>> Hi David,
>>
>> Thanks for this patchset. I am away from my computers for testing but I was
>> wondering if you based your vma rework on Dave's implementation. If so,
>> you may have the same bug that I had with it.
>>
>> I cannot remember the details of the bug, but I could trigger it by
>> rebooting
>> kde around 13 times on radeon. I didn't have this problem with Nouveau.
> It is based on Dave's code, but I rewrote all of it and fixed several
> bugs. For instance, there was a TTM ref-count leak for BOs in TTM core
> and a TTM locking issue. I didn't encounter any bugs so far, but I
> didn't try to restart the xserver 13 times. I will do some more
> stress-tests, but it is quite likely you hit one of those two bugs
> with radeon.

Yeah, the problem I had was related to ref-count and I was trying
to fix the locking too.
>> I am not competent to judge this work but I will try to test it and check
>> with my security tests that I wrote that the problem is indeed solved
>> on nouveau and radeon.
> The changes are actually quite simple. I intentionally kept TTM
> locking as it was before, even though I think we could reduce it now.
> Anyway, I will resend v3 (which now includes tegra and staging) this
> weekend. Hopefully we can get it into linux-next soon.

Very nice! Looking forward to it.


[PATCH v2 00/20] Unified VMA Offset Manager v2 (+Render Node RFC)

2013-07-11 Thread David Herrmann
Hi

On Thu, Jul 11, 2013 at 1:12 PM, Martin Peres  wrote:
> On 07/07/2013 19:17, David Herrmann wrote:
>>
>> Hi
>>
>> This is v2 of the unified VMA offset manager series. The first draft is
>> available at LWN [1]. This series replaces the VMA offset managers in GEM
>> and
>> TTM with a unified implementation.
>>
>> The first 4 patches contain the new VMA offset manager and are the only
>> patches
>> that I propose for mainline inclusion.
>> Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar
>> patches in i915-next and I will rebase them once it is merged. Ignore
>> them if you're not interested.
>> Patches 9-19 implement mmap access control. Comments are welcome! They are
>> intended for mainline inclusion, too, but probably require some more
>> review
>> rounds. I'd really appreciate if driver authors could comment on the
>> implementation.
>> Patch 20 shows the main motivation behind this whole series: render nodes.
>> It is
>> marked RFC and I will resend once the VMA manager is merged upstream. Feel
>> free
>> to test it.
>>
>> One more note regarding DRM-Master: Render-clients are independent of
>> DRM-Master
>> (see the DocBook documentation). It really doesn't make sense to
>> create/bind a
>> DRM-Master object to render-clients. However, a lot of core DRM code
>> depends on
>>   ->master != NULL. Drivers need to take care not to call into those core
>> modules
>> if they implement DRIVER_RENDER.
>> I plan on removing multiple-master-support in the next series. So there
>> will be
>> only one global master and each open-file is bound to it. This will make
>> it very
>> easy to phase out "drm_master". The planned "modeset" nodes provide a nice
>> replacement. If anyone knows a **currently used** use-case for
>> multiple-masters,
>> please tell me. I couldn't find one that _actually works_.
>> (SetMaster+DropMaster
>> will obviously stay functional even with drm_master removed).
>>
>>
>> (series available at: https://github.com/dvdhrm/linux/tree/rnodes)
>>
>> Comments welcome!
>> Cheers
>> David
>
> Hi David,
>
> Thanks for this patchset. I am away from my computers for testing but I was
> wondering if you based your vma rework on Dave's implementation. If so,
> you may have the same bug that I had with it.
>
> I cannot remember the details of the bug, but I could trigger it by
> rebooting
> kde around 13 times on radeon. I didn't have this problem with Nouveau.

It is based on Dave's code, but I rewrote all of it and fixed several
bugs. For instance, there was a TTM ref-count leak for BOs in TTM core
and a TTM locking issue. I didn't encounter any bugs so far, but I
didn't try to restart the xserver 13 times. I will do some more
stress-tests, but it is quite likely you hit one of those two bugs
with radeon.

> I am not competent to judge this work but I will try to test it and check
> with my security tests that I wrote that the problem is indeed solved
> on nouveau and radeon.

The changes are actually quite simple. I intentionally kept TTM
locking as it was before, even though I think we could reduce it now.
Anyway, I will resend v3 (which now includes tegra and staging) this
weekend. Hopefully we can get it into linux-next soon.

Thanks!
David

> Looking forward to seeing your proposition for the userspace :)
>
> Cheers,
> Martin


[PATCH v2 00/20] Unified VMA Offset Manager v2 (+Render Node RFC)

2013-07-11 Thread Martin Peres
On 07/07/2013 19:17, David Herrmann wrote:
> Hi
>
> This is v2 of the unified VMA offset manager series. The first draft is
> available at LWN [1]. This series replaces the VMA offset managers in GEM and
> TTM with a unified implementation.
>
> The first 4 patches contain the new VMA offset manager and are the only 
> patches
> that I propose for mainline inclusion.
> Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar
> patches in i915-next and I will rebase them once it is merged. Ignore
> them if you're not interested.
> Patches 9-19 implement mmap access control. Comments are welcome! They are
> intended for mainline inclusion, too, but probably require some more review
> rounds. I'd really appreciate if driver authors could comment on the
> implementation.
> Patch 20 shows the main motivation behind this whole series: render nodes. It 
> is
> marked RFC and I will resend once the VMA manager is merged upstream. Feel 
> free
> to test it.
>
> One more note regarding DRM-Master: Render-clients are independent of 
> DRM-Master
> (see the DocBook documentation). It really doesn't make sense to create/bind a
> DRM-Master object to render-clients. However, a lot of core DRM code depends 
> on
>   ->master != NULL. Drivers need to take care not to call into those core 
> modules
> if they implement DRIVER_RENDER.
> I plan on removing multiple-master-support in the next series. So there will 
> be
> only one global master and each open-file is bound to it. This will make it 
> very
> easy to phase out "drm_master". The planned "modeset" nodes provide a nice
> replacement. If anyone knows a **currently used** use-case for 
> multiple-masters,
> please tell me. I couldn't find one that _actually works_. 
> (SetMaster+DropMaster
> will obviously stay functional even with drm_master removed).
>
>
> (series available at: https://github.com/dvdhrm/linux/tree/rnodes)
>
> Comments welcome!
> Cheers
> David
Hi David,

Thanks for this patchset. I am away from my computers for testing but I was
wondering if you based your vma rework on Dave's implementation. If so,
you may have the same bug that I had with it.

I cannot remember the details of the bug, but I could trigger it by 
rebooting
kde around 13 times on radeon. I didn't have this problem with Nouveau.

I am not competent to judge this work but I will try to test it and check
with my security tests that I wrote that the problem is indeed solved
on nouveau and radeon.

Looking forward to seeing your proposition for the userspace :)

Cheers,
Martin


Re: [PATCH v2 00/20] Unified VMA Offset Manager v2 (+Render Node RFC)

2013-07-11 Thread David Herrmann
Hi

On Thu, Jul 11, 2013 at 1:12 PM, Martin Peres martin.pe...@labri.fr wrote:
 On 07/07/2013 19:17, David Herrmann wrote:

 Hi

 This is v2 of the unified VMA offset manager series. The first draft is
 available at LWN [1]. This series replaces the VMA offset managers in GEM
 and
 TTM with a unified implementation.

 The first 4 patches contain the new VMA offset manager and are the only
 patches
 that I propose for mainline inclusion.
 Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar
 patches in i915-next and I will rebase them once it is merged. Ignore
 them if you're not interested.
 Patches 9-19 implement mmap access control. Comments are welcome! They are
 intended for mainline inclusion, too, but probably require some more
 review
 rounds. I'd really appreciate if driver authors could comment on the
 implementation.
 Patch 20 shows the main motivation behind this whole series: render nodes.
 It is
 marked RFC and I will resend once the VMA manager is merged upstream. Feel
 free
 to test it.

 One more note regarding DRM-Master: Render-clients are independent of
 DRM-Master
 (see the DocBook documentation). It really doesn't make sense to
 create/bind a
 DRM-Master object to render-clients. However, a lot of core DRM code
 depends on
   -master != NULL. Drivers need to take care not to call into those core
 modules
 if they implement DRIVER_RENDER.
 I plan on removing multiple-master-support in the next series. So there
 will be
 only one global master and each open-file is bound to it. This will make
 it very
 easy to phase out drm_master. The planned modeset nodes provide a nice
 replacement. If anyone knows a **currently used** use-case for
 multiple-masters,
 please tell me. I couldn't find one that _actually works_.
 (SetMaster+DropMaster
 will obviously stay functional even with drm_master removed).


 (series available at: https://github.com/dvdhrm/linux/tree/rnodes)

 Comments welcome!
 Cheers
 David

 Hi David,

 Thanks for this patchset. I am away from my computers for testing but I was
 wondering if you based your vma rework on Dave's implementation. If so,
 you may have the same bug that I had with it.

 I cannot remember the details of the bug, but I could trigger it by
 rebooting
 kde around 13 times on radeon. I didn't have this problem with Nouveau.

It is based on Dave's code, but I rewrote all of it and fixed several
bugs. For instance, there was a TTM ref-count leak for BOs in TTM core
and a TTM locking issue. I didn't encounter any bugs so far, but I
didn't try to restart the xserver 13 times. I will do some more
stress-tests, but it is quite likely you hit one of those two bugs
with radeon.

 I am not competent to judge this work but I will try to test it and check
 with my security tests that I wrote that the problem is indeed solved
 on nouveau and radeon.

The changes are actually quite simple. I intentionally kept TTM
locking as it was before, even though I think we could reduce it now.
Anyway, I will resend v3 (which now includes tegra and staging) this
weekend. Hopefully we can get it into linux-next soon.

Thanks!
David

 Looking forward to seeing your proposition for the userspace :)

 Cheers,
 Martin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2 00/20] Unified VMA Offset Manager v2 (+Render Node RFC)

2013-07-11 Thread Martin Peres

On 11/07/2013 13:21, David Herrmann wrote:

Hi

On Thu, Jul 11, 2013 at 1:12 PM, Martin Peres martin.pe...@labri.fr wrote:

On 07/07/2013 19:17, David Herrmann wrote:

Hi

This is v2 of the unified VMA offset manager series. The first draft is
available at LWN [1]. This series replaces the VMA offset managers in GEM
and
TTM with a unified implementation.

The first 4 patches contain the new VMA offset manager and are the only
patches
that I propose for mainline inclusion.
Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar
patches in i915-next and I will rebase them once it is merged. Ignore
them if you're not interested.
Patches 9-19 implement mmap access control. Comments are welcome! They are
intended for mainline inclusion, too, but probably require some more
review
rounds. I'd really appreciate if driver authors could comment on the
implementation.
Patch 20 shows the main motivation behind this whole series: render nodes.
It is
marked RFC and I will resend once the VMA manager is merged upstream. Feel
free
to test it.

One more note regarding DRM-Master: Render-clients are independent of
DRM-Master
(see the DocBook documentation). It really doesn't make sense to
create/bind a
DRM-Master object to render-clients. However, a lot of core DRM code
depends on
   -master != NULL. Drivers need to take care not to call into those core
modules
if they implement DRIVER_RENDER.
I plan on removing multiple-master-support in the next series. So there
will be
only one global master and each open-file is bound to it. This will make
it very
easy to phase out drm_master. The planned modeset nodes provide a nice
replacement. If anyone knows a **currently used** use-case for
multiple-masters,
please tell me. I couldn't find one that _actually works_.
(SetMaster+DropMaster
will obviously stay functional even with drm_master removed).


(series available at: https://github.com/dvdhrm/linux/tree/rnodes)

Comments welcome!
Cheers
David

Hi David,

Thanks for this patchset. I am away from my computers for testing but I was
wondering if you based your vma rework on Dave's implementation. If so,
you may have the same bug that I had with it.

I cannot remember the details of the bug, but I could trigger it by
rebooting
kde around 13 times on radeon. I didn't have this problem with Nouveau.

It is based on Dave's code, but I rewrote all of it and fixed several
bugs. For instance, there was a TTM ref-count leak for BOs in TTM core
and a TTM locking issue. I didn't encounter any bugs so far, but I
didn't try to restart the xserver 13 times. I will do some more
stress-tests, but it is quite likely you hit one of those two bugs
with radeon.


Yeah, the problem I had was related to ref-count and I was trying
to fix the locking too.

I am not competent to judge this work but I will try to test it and check
with my security tests that I wrote that the problem is indeed solved
on nouveau and radeon.

The changes are actually quite simple. I intentionally kept TTM
locking as it was before, even though I think we could reduce it now.
Anyway, I will resend v3 (which now includes tegra and staging) this
weekend. Hopefully we can get it into linux-next soon.


Very nice! Looking forward to it.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 00/20] Unified VMA Offset Manager v2 (+Render Node RFC)

2013-07-07 Thread David Herrmann
Hi

This is v2 of the unified VMA offset manager series. The first draft is
available at LWN [1]. This series replaces the VMA offset managers in GEM and
TTM with a unified implementation.

The first 4 patches contain the new VMA offset manager and are the only patches
that I propose for mainline inclusion.
Patches 5-8 clean up drm_mm and are informational-only. Daniel has similar
patches in i915-next and I will rebase them once it is merged. Ignore
them if you're not interested.
Patches 9-19 implement mmap access control. Comments are welcome! They are
intended for mainline inclusion, too, but probably require some more review
rounds. I'd really appreciate if driver authors could comment on the
implementation.
Patch 20 shows the main motivation behind this whole series: render nodes. It is
marked RFC and I will resend once the VMA manager is merged upstream. Feel free
to test it.

One more note regarding DRM-Master: Render-clients are independent of DRM-Master
(see the DocBook documentation). It really doesn't make sense to create/bind a
DRM-Master object to render-clients. However, a lot of core DRM code depends on
 ->master != NULL. Drivers need to take care not to call into those core modules
if they implement DRIVER_RENDER.
I plan on removing multiple-master-support in the next series. So there will be
only one global master and each open-file is bound to it. This will make it very
easy to phase out "drm_master". The planned "modeset" nodes provide a nice
replacement. If anyone knows a **currently used** use-case for multiple-masters,
please tell me. I couldn't find one that _actually works_. (SetMaster+DropMaster
will obviously stay functional even with drm_master removed).


(series available at: https://github.com/dvdhrm/linux/tree/rnodes)

Comments welcome!
Cheers
David


Changes in v2:
 - Drop drm_mm_init() fix (already applied in drm-next)
 - Drop drm_mm_node_linked() in favor of drm_mm_node_allocated()
 - Adjust DocBook integration
 - Rebased on drm-next (and compile tested on ARM)
 - Removed unnecessary likely/unlikely marks (except for rbtree paths)
 - small fixes (see commit messages for more)

New in v2:
 - Cleanup patches for drm_mm. Note that these are marked RFC as Daniel has
   similar patches pending in i915-next. But the patches show that most of
   drm_mm is obsolete and can be removed.
   I will rebase them once i915-next is merged into drm-next and resend.
 - Access Management API: Following Dave's proposal, this series implements
   mmap-offset access managamenet via a "file_priv" lookup-tree for each node.
 - Render-Nodes: Following krh's proposal, I revived render nodes which no
   longer have the mmap security problem. Driver-implementations still missing,
   but you can find working examples for i915 and nouveau on dri-devel or [2].

Still To Do:
 - See whether _DRM_GEM can be dropped. I haven't checked legacy user-space so
   far.
 - Patch #16 (vmwgfx) has a TODO marker. I couldn't figure out whether the
   proposed replacement does the same as the old code (with all the
   side-effects). Would be nice if Thomas could comment on that (CC'ed).
 - Patch #18 (gem mmap) adds filp to all VMAs during open and removes them
   during close. This is not necessary for TTM+GEM drivers which don't use
   gem_mmap(). However, it works for now and I wanted to avoid 20 more patches
   fixing each GEM driver. I will provide that in the final series.


[1] Unified VMA Offset Manager v1: https://lwn.net/Articles/557265/
[2] Render-node driver patches: 
https://gitorious.org/linux-nouveau-pm/linux-nouveau-pm/commits/render_nodes


David Herrmann (20):
  drm: add unified vma offset manager
  drm/gem: convert to new unified vma manager
  drm/ttm: convert to unified vma offset manager
  drm/vma: provide drm_vma_node_unmap() helper
  drm/mm: add "best_match" to drm_mm_insert_node()
  drm/ttm: replace drm_mm_pre_get() by direct alloc
  drm/i915: pre-alloc instead of drm_mm search/get_block
  drm/mm: remove unused API
  drm/vma: add access management helpers
  drm/ast: implement mmap access managament
  drm/cirrus: implement mmap access managament
  drm/mgag200: implement mmap access managament
  drm/nouveau: implement mmap access managament
  drm/radeon: implement mmap access managament
  drm/qxl: implement mmap access managament
  drm/vmwgfx: implement mmap access managament
  drm/ttm: prevent mmap access to unauthorized users
  drm/gem: implement mmap access management
  drm: fix minor number range calculation
  drm: implement render nodes

 Documentation/DocBook/drm.tmpl |  77 ++
 drivers/gpu/drm/Makefile   |   2 +-
 drivers/gpu/drm/ast/ast_drv.c  |   2 +
 drivers/gpu/drm/ast/ast_drv.h  |   4 +
 drivers/gpu/drm/ast/ast_main.c |  17 +-
 drivers/gpu/drm/cirrus/cirrus_drv.h|   4 +
 drivers/gpu/drm/cirrus/cirrus_main.c   |  17 +-
 drivers/gpu/drm/drm_drv.c  |  15 +-
 drivers/gpu/drm/drm_fops.c