linux-kernel - Re: [PATCH] drm/amd/display: Increase frame-larger-than warning limit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 13 Jun 2024 15:22:25 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Palmer Dabbelt <palmer@...osinc.com>
Cc: alexander.deucher@....com, harry.wentland@....com, sunpeng.li@....com,
	Rodrigo.Siqueira@....com, christian.koenig@....com,
	Xinhui.Pan@....com, airlied@...il.com, daniel@...ll.ch,
	Qingqing.Zhuo@....com, hamza.mahfooz@....com, chenhuacai@...nel.org,
	amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH] drm/amd/display: Increase frame-larger-than warning limit

Hi Palmer (and AMD folks),

On Tue, Jun 04, 2024 at 09:04:23AM -0700, Palmer Dabbelt wrote:
> On Mon, 03 Jun 2024 15:29:48 PDT (-0700), nathan@...nel.org wrote:
> > On Thu, May 30, 2024 at 07:57:42AM -0700, Palmer Dabbelt wrote:
> > > From: Palmer Dabbelt <palmer@...osinc.com>
> > > 
> > > I get a handful of build errors along the lines of
> > > 
> > >     linux/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:58:13: error: stack frame size (2352) exceeds limit (2048) in 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
> > >     static void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation(
> > >                 ^
> > >     linux/drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1724:6: error: stack frame size (2096) exceeds limit (2048) in 'dml32_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
> > >     void dml32_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib)
> > >          ^
> > 
> > Judging from the message, this is clang/LLVM? What version?
> 
> Yes, LLVM.  Looks like I'm on 16.0.6.  Probably time for an update, so I'll
> give it a shot.

FWIW, I can reproduce this with tip of tree, I was just curious in case
that ended up mattering.

> > I assume
> > this showed up in 6.10-rc1 because of commit 77acc6b55ae4 ("riscv: add
> > support for kernel-mode FPU"), which allows this driver to be built for
> > RISC-V.
> 
> Seems reasonable.  This didn't show up until post-merge, not 100% sure why.
> I didn't really dig any farther.

Perhaps you fast forwarded your tree to include that commit?

> > Is this allmodconfig or some other configuration?
> 
> IIRC both "allmodconfig" and "allyesconfig" show it, but I don't have a
> build tree sitting around to be 100% sure.

Yeah, allmodconfig triggers it.

I was able to come up with a "trivial" reproducer using cvise (attached
to this mail if you are curious) that has worse stack usage by a rough
factor of 2:

  $ clang --target=riscv64-linux-gnu -O2 -Wall -Wframe-larger-than=512 -c -o /dev/null display_mode_vba_32.i
  display_mode_vba_32.i:598:6: warning: stack frame size (1264) exceeds limit (512) in 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Wframe-larger-than]
    598 | void DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation() {
        |      ^
  1 warning generated.

  $ riscv64-linux-gcc -O2 -Wall -Wframe-larger-than=512 -c -o /dev/null display_mode_vba_32.i
  display_mode_vba_32.i: In function 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation':
  display_mode_vba_32.i:1729:1: warning: the frame size of 528 bytes is larger than 512 bytes [-Wframe-larger-than=]
   1729 | }
        | ^

I have not done too much further investigation but this is almost
certainly the same issue that has come up before [1][2] with the AMD
display code using functions with a large number of parameters, such
that they have to passed on the stack, coupled with inlining (if I
remember correctly, LLVM gives more of an inlining discount the less a
function is used in a file).

While clang does poorly with that code, I am not interested in
continuing to fix this code new hardware revision after new hardware
revision. We could just avoid this code like we do for arm64 for a
similar reason:

diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig
index 5fcd4f778dc3..64df713df878 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -8,7 +8,7 @@ config DRM_AMD_DC
 	depends on BROKEN || !CC_IS_CLANG || ARM64 || RISCV || SPARC64 || X86_64
 	select SND_HDA_COMPONENT if SND_HDA_CORE
 	# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
-	select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!ARM64 || !CC_IS_CLANG)
+	select DRM_AMD_DC_FP if ARCH_HAS_KERNEL_FPU_SUPPORT && (!(ARM64 || RISCV) || !CC_IS_CLANG)
 	help
 	  Choose this option if you want to use the new display engine
 	  support for AMDGPU. This adds required support for Vega and

[1]: https://lore.kernel.org/20231019205117.GA839902@dev-arch.thelio-3990X/
[2]: https://lore.kernel.org/20220830203409.3491379-1-nathan@kernel.org/

Cheers,
Nathan

View attachment "display_mode_vba_32.i" of type "text/plain" (107580 bytes)