The vc4 has a custom API to allow components to register a debugfs file
before the DRM driver has been registered and the debugfs_init hook has
been called.
However, the .late_register hook allows to have the debugfs file creation
deferred after that time already.
Let's remove our custom code to only register later our debugfs entries as
part of either debugfs_init or after it.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-65-maxime@cerno.tech
Our current code now mixes some resources whose lifetime are tied to the
device (clocks, IO mappings, etc.) and some that are tied to the DRM device
(encoder, bridge).
The device one will be freed at unbind time, but the DRM one will only be
freed when the last user of the DRM device closes its file handle.
So we end up with a time window during which we can call the encoder hooks,
but we don't have access to the underlying resources and device.
Let's protect all those sections with drm_dev_enter() and drm_dev_exit() so
that we bail out if we are during that window.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-63-maxime@cerno.tech
Whenever the device and driver are unbound, the main device and all the
subdevices will be removed by calling their unbind() method.
However, the DRM device itself will only be freed when the last user will
have closed it.
It means that there is a time window where the device and its resources
aren't there anymore, but the userspace can still call into our driver.
Fortunately, the DRM framework provides the drm_dev_enter() and
drm_dev_exit() functions to make sure our underlying device is still there
for the section protected by those calls. Let's add them to the VEC driver.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-61-maxime@cerno.tech
The current code will call drm_connector_unregister() and
drm_connector_cleanup() when the device is unbound. However, by then, there
might still be some references held to that connector, including by the
userspace that might still have the DRM device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-60-maxime@cerno.tech
The current code will call drm_encoder_cleanup() when the device is
unbound. However, by then, there might still be some references held to
that encoder, including by the userspace that might still have the DRM
device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-59-maxime@cerno.tech
Our internal structure that stores the DRM entities structure is allocated
through a device-managed kzalloc.
This means that this will eventually be freed whenever the device is
removed. In our case, the most likely source of removal is that the main
device is going to be unbound, and component_unbind_all() is being run.
However, it occurs while the DRM device is still registered, which will
create dangling pointers, eventually resulting in use-after-free.
Switch to a DRM-managed allocation to keep our structure until the DRM
driver doesn't need it anymore.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-57-maxime@cerno.tech
The VC4 VEC driver private structure contains only a pointer to the
encoder and connector it implements. This makes the overall structure
somewhat inconsistent with the rest of the driver, and complicates its
initialisation without any apparent gain.
Let's embed the drm_encoder structure (through the vc4_encoder one) and
drm_connector into struct vc4_vec to fix both issues.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-56-maxime@cerno.tech
Our current code now mixes some resources whose lifetime are tied to the
device (clocks, IO mappings, etc.) and some that are tied to the DRM device
(encoder, bridge).
The device one will be freed at unbind time, but the DRM one will only be
freed when the last user of the DRM device closes its file handle.
So we end up with a time window during which we can call the encoder hooks,
but we don't have access to the underlying resources and device.
Let's protect all those sections with drm_dev_enter() and drm_dev_exit() so
that we bail out if we are during that window.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-54-maxime@cerno.tech
Our internal structure that stores the DRM entities structure is allocated
through a device-managed kzalloc.
This means that this will eventually be freed whenever the device is
removed. In our case, the most likely source of removal is that the main
device is going to be unbound, and component_unbind_all() is being run.
However, it occurs while the DRM device is still registered, which will
create dangling pointers, eventually resulting in use-after-free.
Switch to a DRM-managed allocation to keep our structure until the DRM
driver doesn't need it anymore.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-52-maxime@cerno.tech
Whenever the device and driver are unbound, the main device and all the
subdevices will be removed by calling their unbind() method.
However, the DRM device itself will only be freed when the last user will
have closed it.
It means that there is a time window where the device and its resources
aren't there anymore, but the userspace can still call into our driver.
Fortunately, the DRM framework provides the drm_dev_enter() and
drm_dev_exit() functions to make sure our underlying device is still there
for the section protected by those calls. Let's add them to the HDMI driver.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-48-maxime@cerno.tech
Commit 776efe800f ("drm/vc4: hdmi: Drop devm interrupt handler for
hotplug interrupts") dropped the device-managed interrupt registration
because it was creating bugs and races whenever an interrupt was coming in
while the device was removed.
However, our latest patches to the HDMI controller driver fix this as well,
so we can use device-managed interrupt handlers again.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-46-maxime@cerno.tech
The current code to build the registers set later exposed in debugfs for
the HDMI controller relies on traditional allocations, that are later
free'd as part of the driver unbind hook.
Since krealloc doesn't have a DRM-managed equivalent, let's add an action
to free the buffer later on.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-45-maxime@cerno.tech
The current code to unregister our CEC device needs to be undone manually
when we remove the HDMI driver.
Since the CEC framework will allocate its main structure, and will defer
its deallocation to when the last user will have closed it, we don't really
need to take any particular measure to prevent any use-after-free and can
thus use any managed action.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-43-maxime@cerno.tech
The current code to unregister our ALSA device needs to be undone manually
when we remove the HDMI driver.
Since ALSA doesn't seem to support any mechanism to defer freeing something
until the last user of the ALSA device is gone, we can either use a
device-managed or a DRM-managed action.
The consistent way would be to use a DRM-managed one, just like pretty much
any framework-facing structure should be doing. However, ALSA does a lot of
allocation and registration using device-managed calls. Thus, if we're
going that way, by the time the DRM-managed action would run all of those
allocation would have been freed and we would end up with a use-after-free.
Thus, let's do a device-managed action. It's been tested with KASAN enabled
and doesn't seem to trigger any issue, so it's as good as anything.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-42-maxime@cerno.tech
The current code will call drm_connector_unregister() and
drm_connector_cleanup() when the device is unbound. However, by then, there
might still be some references held to that connector, including by the
userspace that might still have the DRM device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-41-maxime@cerno.tech
The current code will call drm_encoder_cleanup() when the device is
unbound. However, by then, there might still be some references held to
that encoder, including by the userspace that might still have the DRM
device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-40-maxime@cerno.tech
Our internal structure that stores the DRM entities structure is allocated
through a device-managed kzalloc.
This means that this will eventually be freed whenever the device is
removed. In our case, the most likely source of removal is that the main
device is going to be unbound, and component_unbind_all() is being run.
However, it occurs while the DRM device is still registered, which will
create dangling pointers, eventually resulting in use-after-free.
Switch to a DRM-managed allocation to keep our structure until the DRM
driver doesn't need it anymore.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-38-maxime@cerno.tech
The vc4_dsi structure is currently allocated through a device-managed
allocation. This can lead to use-after-free issues however in the unbinding
path since the DRM entities will stick around, but the underlying structure
has been freed.
However, we can't just fix it by using a DRM-managed allocation like we did
for the other drivers since the DSI case is a bit more intricate.
Indeed, the structure will be allocated at probe time, when we don't have a
DRM device yet, to be able to register the DSI bus driver. We will then
reuse it at bind time to register our KMS entities in the framework.
In order to work around both constraints, we can use a kref to track the
users of the structure (DSI host, and KMS), and then put our structure when
the DSI host will have been unregistered, and through a DRM-managed action
that will execute once we won't need the KMS entities anymore.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-36-maxime@cerno.tech
The current code uses a device-managed function to retrieve the next bridge
downstream.
However, that means that it will be removed at unbind time, where the DRM
device is still very much live and might still have some applications that
still have it open.
Switch to a DRM-managed variant to clean everything up once the DRM device
has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-35-maxime@cerno.tech
The current code will call drm_encoder_cleanup() when the device is
unbound. However, by then, there might still be some references held to
that encoder, including by the userspace that might still have the DRM
device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-34-maxime@cerno.tech
The VC4 DSI driver private structure contains only a pointer to the
encoder it implements. This makes the overall structure somewhat
inconsistent with the rest of the driver, and complicates its
initialisation without any apparent gain.
Let's embed the drm_encoder structure (through the vc4_encoder one) into
struct vc4_dsi to fix both issues.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-33-maxime@cerno.tech
Our current code now mixes some resources whose lifetime are tied to the
device (clocks, IO mappings, etc.) and some that are tied to the DRM device
(encoder, bridge).
The device one will be freed at unbind time, but the DRM one will only be
freed when the last user of the DRM device closes its file handle.
So we end up with a time window during which we can call the encoder hooks,
but we don't have access to the underlying resources and device.
Let's protect all those sections with drm_dev_enter() and drm_dev_exit() so
that we bail out if we are during that window.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-32-maxime@cerno.tech
The current code uses a device-managed function to retrieve the next bridge
downstream.
However, that means that it will be removed at unbind time, where the DRM
device is still very much live and might still have some applications that
still have it open.
Switch to a DRM-managed variant to clean everything up once the DRM device
has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-31-maxime@cerno.tech
The current code will call drm_encoder_cleanup() when the device is
unbound. However, by then, there might still be some references held to
that encoder, including by the userspace that might still have the DRM
device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-30-maxime@cerno.tech
Our internal structure that stores the DRM entities structure is allocated
through a device-managed kzalloc.
This means that this will eventually be freed whenever the device is
removed. In our case, the most likely source of removal is that the main
device is going to be unbound, and component_unbind_all() is being run.
However, it occurs while the DRM device is still registered, which will
create dangling pointers, eventually resulting in use-after-free.
Switch to a DRM-managed allocation to keep our structure until the DRM
driver doesn't need it anymore.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-26-maxime@cerno.tech
The VC4 DPI driver private structure contains only a pointer to the
encoder it implements. This makes the overall structure somewhat
inconsistent with the rest of the driver, and complicates its
initialisation without any apparent gain.
Let's embed the drm_encoder structure (through the vc4_encoder one) into
struct vc4_dpi to fix both issues.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-25-maxime@cerno.tech
The current code will call drm_crtc_cleanup() when the device is
unbound. However, by then, there might still be some references held to
that CRTC, including by the userspace that might still have the DRM
device open.
Let's switch to a DRM-managed initialization to clean up after ourselves
only once the DRM device has been last closed.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-23-maxime@cerno.tech
Our internal structure that stores the DRM entities structure is allocated
through a device-managed kzalloc.
This means that this will eventually be freed whenever the device is
removed. In our case, the most likely source of removal is that the main
device is going to be unbound, and component_unbind_all() is being run.
However, it occurs while the DRM device is still registered, which will
create dangling pointers, eventually resulting in use-after-free.
Switch to a DRM-managed allocation to keep our structure until the DRM
driver doesn't need it anymore.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-22-maxime@cerno.tech
When vc4_crtc_bind() fails after vc4_crtc_init() has been called, we have
a loop undoing the plane creation and calling destroy on each plane
registered and matching the possible_crtcs mask.
However, this is redundant with what drm_mode_config_cleanup() is doing, so
let's remove it.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-19-maxime@cerno.tech
When the HVS driver is unbound, a lot of memory allocations in the LBM and
DLIST RAM are still assigned to planes that are still allocated.
Thus, we hit a warning when calling drm_mm_takedown() since the memory pool
is not completely free of allocations.
Let's free all the currently live entries before calling drm_mm_takedown().
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-17-maxime@cerno.tech
Whenever the device and driver are unbound, the main device and all the
subdevices will be removed by calling their unbind() method.
However, the DRM device itself will only be freed when the last user will
have closed it.
It means that there is a time window where the device and its resources
aren't there anymore, but the userspace can still call into our driver.
Fortunately, the DRM framework provides the drm_dev_enter() and
drm_dev_exit() functions to make sure our underlying device is still there
for the section protected by those calls. Let's add them to the HVS driver.
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220711173939.1132294-16-maxime@cerno.tech
