Re: [PATCH v7.2-rc6] drm/amd/display: fix usage of DC_FPU_{BEGIN,END} with PREEMPT_RT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete Fix: Normal deallocation paths (dc_stream_free and dc_plane_state_free) still sleep in atomic context.
- [High] DC_RUN_WITH_PREEMPTION_ENABLED fails to re-enable preemption if FPU blocks are nested.
- [High] DC_RUN_WITH_PREEMPTION_ENABLED corrupts callee-saved FPU registers on ARM64.
--

--- Patch [1]: [PATCH v7.2-rc6] drm/amd/display: fix usage of DC_FPU_{BEGIN,END} with PREEMPT_RT ---
commit e67f6942f0899807d82dcd2276705097e892a34b
Author: Bert Karwatzki <spasswolf@web.de>

drm/amd/display: fix usage of DC_FPU_{BEGIN,END} with PREEMPT_RT

This commit wraps deallocations in dc_create_stream_for_sink and
dc_create_plane_state with DC_RUN_WITH_PREEMPTION_ENABLED. This prevents
"scheduling while atomic" panics on PREEMPT_RT kernels when sleeping
allocations are called from within FPU-guarded regions.

Link: https://lore.kernel.org/lkml/20260723123449.6494-1-spasswolf@web.de/

> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> index 7666cdc78f4ed..a5a304a3f8029 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c

[ ... ]

> @@ -233,8 +233,9 @@ struct dc_stream_state *dc_create_stream_for_sink(
>  
>  fail:
>  	if (stream) {
> -		kfree(stream->update_scratch);
> -		kfree(stream);
> +		if (stream->update_scratch)
> +			DC_RUN_WITH_PREEMPTION_ENABLED(kfree(stream->update_scratch));
> +		DC_RUN_WITH_PREEMPTION_ENABLED(kfree(stream));

[Severity: High]
This isn't a bug introduced by this patch, but will the normal deallocation
paths still sleep in atomic context?

When dc_state_release_phantom_streams_and_planes() is called from
dcn32_internal_validate_bw(), it executes under DC_FP_START where
preemption is disabled. That validation path drops references to phantom
streams and planes, which synchronously invokes dc_stream_free() and
dc_plane_state_free().

Those destructors unconditionally call kfree() and kvfree(). On PREEMPT_RT,
slab allocators use sleepable rt_mutexes, which will still cause a kernel
panic from scheduling while atomic. Is it possible to apply this fix to
those deallocation paths as well?

>  	}
>  
>  	return NULL;
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> index 72845fc788f3f..04982673ffbc9 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c

[ ... ]

> @@ -86,8 +87,8 @@ uint8_t  dc_plane_get_pipe_mask(struct dc_state *dc_state, const struct dc_plane
>   ******************************************************************************/
>  struct dc_plane_state *dc_create_plane_state(const struct dc *dc)
>  {
> -	struct dc_plane_state *plane_state = kvzalloc_obj(*plane_state,
> -							  GFP_ATOMIC);
> +	struct dc_plane_state *plane_state;
> +	DC_RUN_WITH_PREEMPTION_ENABLED(plane_state = kvzalloc_obj(*plane_state, GFP_ATOMIC));

[Severity: High]
This isn't a bug introduced by this patch, but does
DC_RUN_WITH_PREEMPTION_ENABLED() fail to re-enable preemption if FPU blocks
are nested?

In dc_fpu_begin(), preempt_disable() is called unconditionally on every
entry, meaning preempt_count increases with each nesting level. However,
DC_RUN_WITH_PREEMPTION_ENABLED() calls DC_FP_END() exactly once, which
only decrements the recursion depth and calls preempt_enable() once.

If the FPU recursion depth is 2 or more, preemption remains disabled and
the wrapped kvzalloc_obj() will still crash on PREEMPT_RT.


[Severity: High]
This is a pre-existing issue, but can DC_RUN_WITH_PREEMPTION_ENABLED()
corrupt callee-saved FPU registers on ARM64 when used here?

A caller compiled with FPU support, such as
dml2_mall_phantom_streams_and_planes_create(), has live variables in
callee-saved FPU registers (v8-v15 on ARM64). It calls
dc_state_create_phantom_plane() which routes to dc_create_plane_state().

Since dc_create_plane_state() is compiled without FPU support, it doesn't
save v8-v15 to the stack. When DC_RUN_WITH_PREEMPTION_ENABLED() calls
DC_FP_END(), it invokes kernel_neon_end() and enables preemption.

If a context switch occurs while preemption is enabled, the OS scheduler
will not save the thread's FPU state since the kernel has yielded it. The
v8-v15 registers are permanently clobbered, and execution returns to the
caller with garbage FPU values.

>  
>  	if (NULL == plane_state)
>  		return NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807124944.154232-1-spasswolf@web.de?part=1