Re: [PATCH] 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:

New issues:
- [Critical] DC_RUN_WITH_PREEMPTION_ENABLED() silently clobbers callee-saved FPU/SIMD registers on architectures like ARM64.
- [High] DC_RUN_WITH_PREEMPTION_ENABLED() fails to enable preemption if the FPU recursion depth is greater than 1.

Pre-existing issues:
- [High] The teardown paths for streams and planes call kfree() and kvfree() inside FPU-guarded regions without dropping preemption.
--

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

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

This commit addresses "scheduling while atomic" warnings on PREEMPT_RT by
wrapping memory allocations in DC_RUN_WITH_PREEMPTION_ENABLED() in the
plane and stream creation paths.

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
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c

[ ... ]

> @@ -208,13 +208,13 @@ struct dc_stream_state *dc_create_stream_for_sink(
>  	if (sink == NULL)
>  		goto fail;
>  
> -	stream = kzalloc_obj(struct dc_stream_state, GFP_ATOMIC);
> +	DC_RUN_WITH_PREEMPTION_ENABLED(stream = kzalloc_obj(struct dc_stream_state, GFP_ATOMIC));

[Severity: Critical]
Does DC_RUN_WITH_PREEMPTION_ENABLED() silently clobber callee-saved FPU/SIMD
registers on architectures like ARM64?

The AMD display math library relies on FPU calculations. On ARM64, the ABI
designates certain SIMD registers (e.g., d8-d15 or v8-v15) as callee-saved.
When the display math library calls dc_create_stream_for_sink() from within
an FPU-guarded region, the C compiler expects these callee-saved registers
to be preserved.

When DC_RUN_WITH_PREEMPTION_ENABLED() calls DC_FP_END(), it invokes
kernel_neon_end(). This enables preemption and clears the TIF_KERNEL_FPSTATE
flag, but leaves TIF_FOREIGN_FPSTATE set. If a context switch occurs during
the wrapped kzalloc_obj() allocation, fpsimd_thread_switch() checks
TIF_FOREIGN_FPSTATE and skips saving the hardware FPU registers, erroneously
assuming they do not contain valid task state. The hardware FPU registers are
then clobbered by other tasks. When control returns to the math library, its
local floating-point variables stored in callee-saved registers will contain
garbage.

> @@ -229,7 +229,7 @@ struct dc_stream_state *dc_create_stream_for_sink(
>  
>  fail:
>  	if (stream)
> -		kfree(stream);
> +		DC_RUN_WITH_PREEMPTION_ENABLED(kfree(stream));

[Severity: High]
This is a pre-existing issue, but do the teardown paths for streams and planes
still call kfree() and kvfree() inside FPU-guarded regions without dropping
preemption?

For example, phantom planes and streams are destroyed during bandwidth
validation by calling dc_state_release_phantom_streams_and_planes(), such as
from dcn32_internal_validate_bw(). This validation executes entirely inside an
FPU-guarded region created by DC_FP_START() in dcn32_validate_bandwidth(),
which disables preemption.

When the last reference is dropped, the teardown paths like dc_stream_free()
and dc_plane_state_free() execute synchronously. Since they call kfree() and
kvfree() directly without being wrapped in DC_RUN_WITH_PREEMPTION_ENABLED(),
these allocations are freed while preemption is disabled, which can trigger
the exact PREEMPT_RT bug this patch aims to fix.

>  	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
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c

[ ... ]

> @@ -85,8 +86,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]
Does DC_RUN_WITH_PREEMPTION_ENABLED() fail to enable preemption if the FPU
recursion depth is greater than 1?

The FPU protection in the AMD display driver supports nesting via the
fpu_recursion_depth counter. DC_FP_END() only invokes kernel_fpu_end() (and
thus preempt_enable()) if the recursion depth decrements to exactly 0.

If dc_create_plane_state() or dc_create_stream_for_sink() is invoked from a
nested FPU-guarded region where the depth is 2 or more, the
DC_RUN_WITH_PREEMPTION_ENABLED() macro will merely decrement the depth
counter by 1 but leave preemption disabled. The subsequent call to
kvzalloc_obj() or kfree() will then execute with preemption still disabled,
which will still trigger a scheduling while atomic panic on PREEMPT_RT.

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

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