Commit Graph

10 Commits

Author SHA1 Message Date
Colin Ian King
918d89bb1a gpu: drm/lease: fix spelling mistake, EACCESS -> EACCES
Trivial fix to a spelling mistake of the error access name EACCESS,
rename to EACCES

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20181026180512.4908-1-colin.king@canonical.com
2018-10-29 16:20:16 +01:00
Chris Wilson
69fdf4206a drm: Differentiate the lack of an interface from invalid parameter
If the ioctl is not supported on a particular piece of HW/driver
combination, report ENOTSUP (aka EOPNOTSUPP) so that it can be easily
distinguished from both the lack of the ioctl and from a regular invalid
parameter.

v2: Across all the kms ioctls we had a mixture of reporting EINVAL,
ENODEV and a few ENOTSUPP (most where EINVAL) for a failed
drm_core_check_feature(). Update everybody to report ENOTSUPP.

v3: ENOTSUPP is an internal errno! It's value (524) does not correspond
to a POSIX errno, the one we want is ENOTSUP. However,
uapi/asm-generic/errno.h doesn't include ENOTSUP but man errno says

	"ENOTSUP and EOPNOTSUPP have the same value on Linux,
	but according to POSIX.1 these error values should be
	distinct."

so use EOPNOTSUPP as its equivalent.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> #v2
Link: https://patchwork.freedesktop.org/patch/msgid/20180913192050.24812-1-chris@chris-wilson.co.uk
2018-09-14 17:29:47 +01:00
Al Viro
19f391eb05 turn filp_clone_open() into inline wrapper for dentry_open()
it's exactly the same thing as
	dentry_open(&file->f_path, file->f_flags, file->f_cred)

... and rename it to file_clone_open(), while we are at it.
'filp' naming convention is bogus; sure, it's "file pointer",
but we generally don't do that kind of Hungarian notation.
Some of the instances have too many callers to touch, but this
one has only two, so let's sanitize it while we can...

Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
2018-07-10 23:29:03 -04:00
Al Viro
b4e7a7a88b drm_mode_create_lease_ioctl(): fix open-coded filp_clone_open()
Failure of ->open() should *not* be followed by fput().  Fixed by
using filp_clone_open(), which gets the cleanups right.

Cc: stable@vger.kernel.org
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
2018-07-10 23:29:03 -04:00
Arushi Singhal
aec06c7606 gpu: drm/lease:: Use list_{next/prev}_entry instead of list_entry
It's better to use list_entry instead of list_{next/prev}_entry
as it makes the code more clear to read.
This patch replace list_entry with list_{next/prev}_entry.

Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/1522000893-5331-2-git-send-email-arushisinghal19971997@gmail.com
2018-03-26 10:38:06 +02:00
Chris Wilson
7591844c19 drm: Fix kerneldoc warnings for drm_lease
drivers/gpu/drm/drm_lease.c:56: warning: No description found for parameter 'lessee_id'
drivers/gpu/drm/drm_lease.c:56: warning: Excess function parameter 'id' description in '_drm_find_lessee'
drivers/gpu/drm/drm_lease.c:114: warning: No description found for parameter 'file_priv'
drivers/gpu/drm/drm_lease.c:114: warning: Excess function parameter 'master' description in '_drm_lease_held'
drivers/gpu/drm/drm_lease.c:134: warning: No description found for parameter 'file_priv'
drivers/gpu/drm/drm_lease.c:134: warning: Excess function parameter 'master' description in 'drm_lease_held'
drivers/gpu/drm/drm_lease.c:158: warning: No description found for parameter 'crtcs_in'
drivers/gpu/drm/drm_lease.c:158: warning: Excess function parameter 'crtcs' description in 'drm_lease_filter_crtcs'
drivers/gpu/drm/drm_lease.c:311: warning: No description found for parameter 'top'
drivers/gpu/drm/drm_lease.c:311: warning: Excess function parameter 'master' description in '_drm_lease_revoke'
drivers/gpu/drm/drm_lease.c:494: warning: No description found for parameter 'lessor_priv'
drivers/gpu/drm/drm_lease.c:494: warning: Excess function parameter 'file_priv' description in 'drm_mode_create_lease_ioctl'
drivers/gpu/drm/drm_lease.c:672: warning: No description found for parameter 'lessee_priv'
drivers/gpu/drm/drm_lease.c:672: warning: Excess function parameter 'file_priv' description in 'drm_mode_get_lease_ioctl'
drivers/gpu/drm/drm_lease.c:733: warning: No description found for parameter 'lessor_priv'
drivers/gpu/drm/drm_lease.c:733: warning: Excess function parameter 'file_priv' description in 'drm_mode_revoke_lease_ioctl'

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20180208110817.30057-1-chris@chris-wilson.co.uk
2018-02-19 10:49:59 +01:00
Keith Packard
d2a48e5254 drm: move lease init after validation in drm_lease_create
Patch bd36d3bab2 fixed a deadlock in the
failure path of drm_lease_create. This made the partially initialized
lease object visible for a short window of time.

To avoid having the lessee state appear transiently, I've rearranged
the code so that the lessor fields are not filled in until the
parameters are all validated and the function will succeed.

Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20171221065424.1304-1-keithp@keithp.com
2017-12-21 09:49:40 +01:00
Marius Vlad
bd36d3bab2 drm/drm_lease: Prevent deadlock in case drm_lease_create() fails
This case can been seen when creating the lease with the same objects passed.

[  605.515097] 2 locks held by testapp/3337:
[  605.519027]  #0:  (&dev->mode_config.idr_mutex){......}, at: [<ffff0000085f1664>] drm_mode_create_lease_ioctl+0x384/0x858
[  605.530045]  #1:  (&dev->mode_config.idr_mutex){......}, at: [<ffff0000085f11bc>] drm_lease_destroy+0x2c/0x110

Which was causing the process to hang:

[  605.398827] [<ffff0000080856cc>] __switch_to+0x94/0xa8
[  605.404030] [<ffff000008c05d00>] __schedule+0x1b0/0x698
[  605.409322] [<ffff000008c06224>] schedule+0x3c/0xa8
[  605.414260] [<ffff000008c06628>] schedule_preempt_disabled+0x20/0x38
[  605.420677] [<ffff000008c07370>] mutex_lock_nested+0x158/0x340
[  605.426572] [<ffff0000085f11bc>] drm_lease_destroy+0x2c/0x110
[  605.432389] [<ffff0000085cecf0>] drm_master_put+0xc0/0xc8
[  605.437845] [<ffff0000085f175c>] drm_mode_create_lease_ioctl+0x47c/0x858
[  605.444612] [<ffff0000085d4460>] drm_ioctl+0x198/0x448
[  605.449811] [<ffff000008201134>] do_vfs_ioctl+0xa4/0x748
[  605.455192] [<ffff000008201864>] SyS_ioctl+0x8c/0xa0
[  605.460216] [<ffff000008082f4c>] __sys_trace_return+0x0/0x4

drm_mode_create_lease_ioctl() calls drm_lease_create() which acquires a lock
on dev->mode_config.idr_mutex. In case of failure, drm_lease_create() calls
drm_master_put() which in turn tries to acquire the same lock when calling
drm_lease_destroy().

v2: - Reverse the order at exit in case of fail, so that unlocking takes place
before dropping the reference.
    - Include detail information about deadlock (Daniel Vetter)

Signed-off-by: Marius Vlad <marius-cristian.vlad@nxp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20171213181048.32719-1-marius-cristian.vlad@nxp.com
2017-12-14 08:25:37 +01:00
Keith Packard
62884cd386 drm: Add four ioctls for managing drm mode object leases [v7]
drm_mode_create_lease

	Creates a lease for a list of drm mode objects, returning an
	fd for the new drm_master and a 64-bit identifier for the lessee

drm_mode_list_lesees

	List the identifiers of the lessees for a master file

drm_mode_get_lease

	List the leased objects for a master file

drm_mode_revoke_lease

	Erase the set of objects managed by a lease.

This should suffice to at least create and query leases.

Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:

 * query ioctls only query the master associated with
   the provided file.

 * 'mask_lease' value has been removed

 * change ioctl has been removed.

Changes for v3 suggested in part by Dave Airlie <airlied@gmail.com>

 * Add revoke ioctl.

Changes for v4 suggested by Dave Airlie <airlied@gmail.com>

 * Expand on the comment about the magic use of &drm_lease_idr_object
 * Pad lease ioctl structures to align on 64-bit boundaries

Changes for v5 suggested by Dave Airlie <airlied@gmail.com>

 * Check for non-negative object_id in create_lease to avoid debug
   output from the kernel.

Changes for v6 provided by Dave Airlie <airlied@gmail.com>

 * For non-universal planes add primary/cursor planes to lease

   If we aren't exposing universal planes to this userspace client,
   and it requests a lease on a crtc, we should implicitly export the
   primary and cursor planes for the crtc.

   If the lessee doesn't request universal planes, it will just see
   the crtc, but if it does request them it will then see the plane
   objects as well.

   This also moves the object look ups earlier as a side effect, so
   we'd exit the ioctl quicker for non-existant objects.

 * Restrict leases to crtc/connector/planes.

   This only allows leasing for objects we wish to allow.

Changes for v7 provided by Dave Airlie <airlied@gmail.com>

 * Check pad args are 0
 * Check create flags and object count are valid.
 * Check return from fd allocation
 * Refactor lease idr setup and add some simple validation
 * Use idr_mutex uniformly (Keith)

Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
2017-10-25 16:31:30 +10:00
Keith Packard
2ed077e467 drm: Add drm_object lease infrastructure [v5]
This provides new data structures to hold "lease" information about
drm mode setting objects, and provides for creating new drm_masters
which have access to a subset of the available drm resources.

An 'owner' is a drm_master which is not leasing the objects from
another drm_master, and hence 'owns' them.

A 'lessee' is a drm_master which is leasing objects from some other
drm_master. Each lessee holds the set of objects which it is leasing
from the lessor.

A 'lessor' is a drm_master which is leasing objects to another
drm_master. This is the same as the owner in the current code.

The set of objects any drm_master 'controls' is limited to the set of
objects it leases (for lessees) or all objects (for owners).

Objects not controlled by a drm_master cannot be modified through the
various state manipulating ioctls, and any state reported back to user
space will be edited to make them appear idle and/or unusable. For
instance, connectors always report 'disconnected', while encoders
report no possible crtcs or clones.

The full list of lessees leasing objects from an owner (either
directly, or indirectly through another lessee), can be searched from
an idr in the drm_master of the owner.

Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:

* Sub-leasing has been disabled.

* BUG_ON for lock checking replaced with lockdep_assert_held

* 'change' ioctl has been removed.

* Leased objects can always be controlled by the lessor; the
  'mask_lease' flag has been removed

* Checking for leased status has been simplified, replacing
  the drm_lease_check function with drm_lease_held.

Changes in v3, some suggested by Dave Airlie <airlied@gmail.com>

* Add revocation. This allows leases to be effectively revoked by
  removing all of the objects they have access to. The lease itself
  hangs around as it's hanging off a file.

* Free the leases IDR when the master is destroyed

* _drm_lease_held should look at lessees, not lessor

* Allow non-master files to check for lease status

Changes in v4, suggested by Dave Airlie <airlied@gmail.com>

* Formatting and whitespace changes

Changes in v5 (airlied)

* check DRIVER_MODESET before lease destroy call
* check DRIVER_MODESET for lease revoke (Chris)
* Use idr_mutex uniformly for all lease elements of struct drm_master. (Keith)

Signed-off-by: Keith Packard <keithp@keithp.com>
2017-10-25 16:31:29 +10:00