With MST, we now have connector hotplugging, yey! Pretty easy to use in
user-space, but introduces some nasty races:
* If a connector is removed and added again while a compositor is in
background, it will get the same ID again. If the compositor wakes up,
it cannot know whether its the same connector or a new one, thus they
must re-read EDID information, etc.
* possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if
an object is removed and a new one is added, those bitmasks are invalid
and must be refreshed. This currently does not affect connectors, but
only crtcs and encoders, but it's only a matter of time when this will
change.
The easiest way to protect against this, is to not recylce modeset IDs.
Instead, always allocate a new, higher ID. All ioctls that affect modeset
objects, take IDs. Thus, during hotplug races, those ioctls will simply
fail if invalid IDs are passed. They will no longer silently run on a
newly hotplugged object.
Furthermore, hotplug-races during state sync can now be easily detected. A
call to GET_RESOURCES returns a list of available IDs atomically.
User-space can now start fetching all those objects via GET_* ioctls. If
any of those fails, they know that the given object was unplugged. Thus,
the "possible_*" bit-fields are invalidated. User-space can now decide
whether to restart the sync all over or wait for the 'change' uevent that
is sent on modeset object modifications (or, well, is supposed to be sent
and probably will be at some point).
With this in place, modeset object hotplugging should work fine for all
modeset objects in the KMS API.
CC'ing stable so we can rely on all kernels with MST to not recycle IDs.
Cc: <stable@xxxxxxxxxxxxxxx> # 3.16+
Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
---
drivers/gpu/drm/drm_crtc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 97eba56..ab0fe6d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -286,7 +286,7 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
idr_preload(GFP_KERNEL);
spin_lock_irq(&dev->mode_config.idr_lock);
- ret = idr_alloc(&dev->mode_config.crtc_idr, register_obj ? obj : NULL, 1, 0, GFP_NOWAIT);
+ ret = idr_alloc_cyclic(&dev->mode_config.crtc_idr, register_obj ? obj : NULL, 1, 0, GFP_NOWAIT);
if (ret >= 0) {
/*
* Set up the object linking under the protection of the idr
--
2.1.0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
