Re: [PATCH] Enable '-Werror' by default for all kernel builds

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On Sun, Sep 05, 2021 at 11:24:05AM -0700, Linus Torvalds wrote:
> ... but make it a config option so that broken environments can disable
> it when required.
> 
> We really should always have a clean build, and will disable specific
> over-eager warnings as required, if we can't fix them.  But while I
> fairly religiously enforce that in my own tree, it doesn't get enforced
> by various build robots that don't necessarily report warnings.
> 
> So this just makes '-Werror' a default compiler flag, but allows people
> to disable it for their configuration if they have some particular
> issues.
> 
> Occasionally, new compiler versions end up enabling new warnings, and it
> can take a while before we have them fixed (or the warnings disabled if
> that is what it takes), so the config option allows for that situation.
> 
> Hopefully this will mean that I get fewer pull requests that have new
> warnings that were not noticed by various automation we have in place.
> 
> Knock wood.
> 

I guess the good news is that some builds still pass.

Build results:
	total: 153 pass: 89 fail: 64
Failed builds:
	alpha:defconfig
	alpha:allmodconfig
	arcv2:defconfig
	arcv2:axs103_defconfig
	arcv2:vdk_hs38_smp_defconfig
	arm:s3c2410_defconfig
	arm:ixp4xx_defconfig
	arm:omap1_defconfig
	arm:footbridge_defconfig
	arm:keystone_defconfig
	arm:vexpress_defconfig
	arm:imx_v4_v5_defconfig
	arm:s3c6400_defconfig
	arm:s5pv210_defconfig
	arm:integrator_defconfig
	arm:pxa910_defconfig
	arm:clps711x_defconfig
	csky:defconfig
	h8300:edosk2674_defconfig
	h8300:h8300h-sim_defconfig
	h8300:h8s-sim_defconfig
	hexagon:defconfig
	i386:allyesconfig
	i386:allmodconfig
	ia64:defconfig
	m68k:defconfig
	m68k:allmodconfig
	m68k:sun3_defconfig
	m68k_nommu:m5272c3_defconfig
	m68k_nommu:m5307c3_defconfig
	m68k_nommu:m5249evb_defconfig
	m68k_nommu:m5407c3_defconfig
	m68k_nommu:m5475evb_defconfig
	microblaze:mmu_defconfig
	mips:allmodconfig
	mips:bcm63xx_defconfig
	mips:e55_defconfig
	mips:malta_defconfig
	nds32:defconfig
	nds32:allmodconfig
	nios2:3c120_defconfig
	parisc:allmodconfig
	parisc:generic-32bit_defconfig
	parisc64:generic-64bit_defconfig
	powerpc:allmodconfig
	powerpc:cell_defconfig
	powerpc:maple_defconfig
	powerpc:ppc6xx_defconfig
	powerpc:mpc83xx_defconfig
	powerpc:tqm8xx_defconfig
	powerpc:83xx/mpc834x_mds_defconfig
	riscv32:allmodconfig
	riscv:allmodconfig
	s390:allmodconfig
	sh:defconfig
	sh:dreamcast_defconfig
	sh:microdev_defconfig
	sh:shx3_defconfig
	sparc32:defconfig
	sparc64:allmodconfig
	sparc64:defconfig
	um:defconfig
	xtensa:defconfig
	xtensa:allmodconfig
Qemu test results:
	total: 479 pass: 340 fail: 139
Failed tests:
	<many>

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Mon, Sep 6, 2021 at 7:26 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Build results:
>         total: 153 pass: 89 fail: 64

Well, that sadly proves the point of that patch. x86-64 may be clean,
because I have required it manually. Others not necessarily so much..

I've got at least one sparc64 fix in my inbox. It _might_ fix some
other cases too (syscall checking), but I suspect it's one of those
"death by a thousand cuts" situations, not just one or two issues that
show up.

Do you end up exposing the errors anywhere where I can take a look?

If some of them are just because of bad tooling on certain
architectures (ie fundamentally "this is unfixable, because we use
gcc-XYZ that just always causes warnings") then those we could/should
just disable -Werror for those and forget about them.

But hopefully most cases are just "people haven't cared enough" and
easily fixed.

              Linus

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Mon, Sep 6, 2021 at 9:12 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I've got at least one sparc64 fix in my inbox. It _might_ fix some
> other cases too (syscall checking), but I suspect it's one of those
> "death by a thousand cuts" situations, not just one or two issues that
> show up.

I pushed out that "don't make the syscall checking produce errors from
warnings" patch by Stephen Rothwell.

One down, N to go.

                 Linus

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/6/21 9:12 AM, Linus Torvalds wrote:
> On Mon, Sep 6, 2021 at 7:26 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> Build results:
>>          total: 153 pass: 89 fail: 64
> 
> Well, that sadly proves the point of that patch. x86-64 may be clean,
> because I have required it manually. Others not necessarily so much..
> 
> I've got at least one sparc64 fix in my inbox. It _might_ fix some
> other cases too (syscall checking), but I suspect it's one of those
> "death by a thousand cuts" situations, not just one or two issues that
> show up.
> 
> Do you end up exposing the errors anywhere where I can take a look?
> 

Logs are available from KernelCI.
See https://linux.kernelci.org/job/mainline/
I expect that 0-day will also have a field day.

> If some of them are just because of bad tooling on certain
> architectures (ie fundamentally "this is unfixable, because we use
> gcc-XYZ that just always causes warnings") then those we could/should
> just disable -Werror for those and forget about them.
> 
> But hopefully most cases are just "people haven't cared enough" and
> easily fixed.
> 

We'll see. For my testbed I disabled the new configuration flag
for the time being because its primary focus is boot tests, and
there won't be any boot tests if images fail to build.

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]

[ Adding some subsystem maintainers ]

On Mon, Sep 6, 2021 at 10:06 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> > But hopefully most cases are just "people haven't cared enough" and
> > easily fixed.
>
> We'll see. For my testbed I disabled the new configuration flag
> for the time being because its primary focus is boot tests, and
> there won't be any boot tests if images fail to build.

Sure, reasonable.

I've checked a few of the build errors by doing the appropriate cross
compiles, and it doesn't seem bad - but it does seem like we have a
number of really pointless long-standing warnings that should have
been fixed long ago.

For example, looking at sparc64, there are several build errors due to
those warnings now being fatal:

 - drivers/gpu/drm/ttm/ttm_pool.c:386

   This is a type mismatch error. It looks like __fls() on sparc64
returns 'int'. And the ttm_pool.c code assumes it returns 'unsigned
long'.

   Oddly enough, the very line after that line does "min_t(unsigned
int" to get the types in line.

   So  the immediate reason is "sparc64 is different". But the deeper
reason seems to be that ttm_pool.c has odd type assumptions. But that
warning should have been fixed long ago, either way.

   Christian/Huang? I get the feeling that both lines in that file
should use the min_t(). Hmm?

 - drivers/input/joystick/analog.c:160

   #warning Precise timer not defined for this architecture.

   Unfortunate. I suspect that warning just has to be removed. It has
never caused anything to be fixed, it's old to the point of predating
the git history. Dmitry?

 - at least a couple of stringop-overread errors. Attached is a
possible for for one of them.

The stringop overread is odd, because another one of them is

   fs/qnx4/dir.c: In function ‘qnx4_readdir’:
   fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds
source size 16 [-Werror=stringop-overread]
      51 |                         size = strnlen(de->di_fname, size);
         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~

but I'm not seeing why that one happens on sparc64, but not on arm64
or x86-64. There doesn't seem to be anything architecture-specific
anywhere in that area.

Funky.

Davem - attached patch compiles cleanly for me, but I'm not sure it's
necessarily the right thing to do, and I didn't check the code
generation. Maybe it screws up. Can somebody test on sparc64 and
perhaps think about it more than I did?

               Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 741 bytes --]

 arch/sparc/kernel/mdesc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c
index 8e645ddac58e..30f171b7b00c 100644
--- a/arch/sparc/kernel/mdesc.c
+++ b/arch/sparc/kernel/mdesc.c
@@ -39,6 +39,7 @@ struct mdesc_hdr {
 	u32	node_sz; /* node block size */
 	u32	name_sz; /* name block size */
 	u32	data_sz; /* data block size */
+	char	data[];
 } __attribute__((aligned(16)));
 
 struct mdesc_elem {
@@ -612,7 +613,7 @@ EXPORT_SYMBOL(mdesc_get_node_info);
 
 static struct mdesc_elem *node_block(struct mdesc_hdr *mdesc)
 {
-	return (struct mdesc_elem *) (mdesc + 1);
+	return (struct mdesc_elem *) mdesc->data;
 }
 
 static void *name_block(struct mdesc_hdr *mdesc)

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On Mon, Sep 06, 2021 at 04:06:04PM -0700, Linus Torvalds wrote:
> [ Adding some subsystem maintainers ]
> 
> On Mon, Sep 6, 2021 at 10:06 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > > But hopefully most cases are just "people haven't cared enough" and
> > > easily fixed.
> >
> > We'll see. For my testbed I disabled the new configuration flag
> > for the time being because its primary focus is boot tests, and
> > there won't be any boot tests if images fail to build.
> 
> Sure, reasonable.
> 
> I've checked a few of the build errors by doing the appropriate cross
> compiles, and it doesn't seem bad - but it does seem like we have a
> number of really pointless long-standing warnings that should have
> been fixed long ago.
> 
> For example, looking at sparc64, there are several build errors due to
> those warnings now being fatal:
> 
>  - drivers/gpu/drm/ttm/ttm_pool.c:386
> 
>    This is a type mismatch error. It looks like __fls() on sparc64
> returns 'int'. And the ttm_pool.c code assumes it returns 'unsigned
> long'.
> 
>    Oddly enough, the very line after that line does "min_t(unsigned
> int" to get the types in line.
> 
>    So  the immediate reason is "sparc64 is different". But the deeper
> reason seems to be that ttm_pool.c has odd type assumptions. But that
> warning should have been fixed long ago, either way.
> 
>    Christian/Huang? I get the feeling that both lines in that file
> should use the min_t(). Hmm?
> 
>  - drivers/input/joystick/analog.c:160
> 
>    #warning Precise timer not defined for this architecture.
> 
>    Unfortunate. I suspect that warning just has to be removed. It has
> never caused anything to be fixed, it's old to the point of predating
> the git history. Dmitry?
> 
My solution would be to just remove the old code (that isn't using ktime)
including the module parameter that disables it. Sure, we want to be
backward compatible, but that code is 15+ years old and should really be
retired.

>  - at least a couple of stringop-overread errors. Attached is a
> possible for for one of them.
> 
> The stringop overread is odd, because another one of them is
> 
>    fs/qnx4/dir.c: In function ‘qnx4_readdir’:
>    fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds
> source size 16 [-Werror=stringop-overread]
>       51 |                         size = strnlen(de->di_fname, size);
>          |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> but I'm not seeing why that one happens on sparc64, but not on arm64
> or x86-64. There doesn't seem to be anything architecture-specific
> anywhere in that area.
> 
> Funky.
> 
Not really. That is because de->di_fname is always 16 bytes but size
can be 48 if the node is really a link. The use of de is overloaded
in that case; de is struct qnx4_inode_entry (where di_fname is 16 bytes)
but the actual data is struct qnx4_link_info where the name is 48 bytes
long. A possible fix (compile tested only) is below.

I think the warning/error is only reported with gcc 11.x. Do you possibly
use an older compiler for x86/arm64 ?

Anyway, below is a partial list of build errors I have seen. Some of
them are easy to fix (such as the ones due to unused functions),
but others seem to be tricky.

Guenter

---
diff --git a/fs/qnx4/dir.c b/fs/qnx4/dir.c
index a6ee23aadd28..f75dcadd98e5 100644
--- a/fs/qnx4/dir.c
+++ b/fs/qnx4/dir.c
@@ -44,20 +44,17 @@ static int qnx4_readdir(struct file *file, struct dir_context *ctx)
 				continue;
 			if (!(de->di_status & (QNX4_FILE_USED|QNX4_FILE_LINK)))
 				continue;
-			if (!(de->di_status & QNX4_FILE_LINK))
-				size = QNX4_SHORT_NAME_MAX;
-			else
-				size = QNX4_NAME_MAX;
-			size = strnlen(de->di_fname, size);
-			QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, de->di_fname));
-			if (!(de->di_status & QNX4_FILE_LINK))
+			if (!(de->di_status & QNX4_FILE_LINK)) {
+				size = strnlen(de->di_fname, QNX4_SHORT_NAME_MAX);
 				ino = blknum * QNX4_INODES_PER_BLOCK + ix - 1;
-			else {
+			} else {
 				le  = (struct qnx4_link_info*)de;
+				size = strnlen(le->dl_fname, QNX4_NAME_MAX);
 				ino = ( le32_to_cpu(le->dl_inode_blk) - 1 ) *
 					QNX4_INODES_PER_BLOCK +
 					le->dl_inode_ndx;
 			}
+			QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, de->di_fname));
 			if (!dir_emit(ctx, de->di_fname, size, ino, DT_UNKNOWN)) {
 				brelse(bh);
 				return 0;

---
alpha.log:arch/alpha/kernel/setup.c:493:13: error: 'strcmp' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
alpha.log:drivers/atm/ambassador.c:1747:58: error: passing argument 1 of 'virt_to_bus' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]
alpha.log:drivers/gpu/drm/rockchip/cdn-dp-core.c:1126:12: error: 'cdn_dp_resume' defined but not used [-Werror=unused-function]
alpha.log:drivers/net/ethernet/3com/3c515.c:1053:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
alpha.log:drivers/net/ethernet/amd/ni65.c:751:37: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
alpha.log:drivers/net/hamradio/6pack.c:71:41: error: unsigned conversion from 'int' to 'unsigned char' changes value from '256' to '0' [-Werror=overflow]
alpha.log:drivers/net/wan/lmc/lmc_main.c:1782:50: error: passing argument 1 of 'virt_to_bus' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]
alpha.log:drivers/net/wan/lmc/lmc_main.c:1791:53: error: passing argument 1 of 'virt_to_bus' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]
alpha.log:drivers/net/wan/lmc/lmc_main.c:1793:51: error: passing argument 1 of 'virt_to_bus' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]
alpha.log:drivers/net/wan/lmc/lmc_main.c:1804:50: error: passing argument 1 of 'virt_to_bus' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]
alpha.log:drivers/net/wan/lmc/lmc_main.c:1806:50: error: passing argument 1 of 'virt_to_bus' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]
alpha.log:drivers/net/wan/lmc/lmc_main.c:1807:51: error: passing argument 1 of 'virt_to_bus' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]
alpha.log:drivers/spi/spi-tegra20-slink.c:1188:12: error: 'tegra_slink_runtime_suspend' defined but not used [-Werror=unused-function]
alpha.log:drivers/spi/spi-tegra20-slink.c:1200:12: error: 'tegra_slink_runtime_resume' defined but not used [-Werror=unused-function]
alpha.log:fs/qnx4/dir.c:51:32: error: 'strnlen' specified bound 48 exceeds source size 16 [-Werror=stringop-overread]
m68k.log:./arch/m68k/include/asm/raw_io.h:20:19: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
m68k.log:./arch/m68k/include/asm/raw_io.h:30:32: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
m68k.log:./arch/m68k/include/asm/string.h:72:25: error: '__builtin_memcpy' reading 6 bytes from a region of size 0 [-Werror=stringop-overread]
m68k.log:arch/m68k/mvme147/config.c:174:2: error: #warning check me! [-Werror=cpp]
m68k.log:arch/m68k/mvme16x/config.c:439:2: error: #warning check me! [-Werror=cpp]
m68k.log:drivers/gpu/drm/rockchip/cdn-dp-core.c:1126:12: error: 'cdn_dp_resume' defined but not used [-Werror=unused-function]
m68k.log:drivers/input/joystick/analog.c:160:2: error: #warning Precise timer not defined for this architecture. [-Werror=cpp]
m68k.log:drivers/spi/spi-tegra20-slink.c:1188:12: error: 'tegra_slink_runtime_suspend' defined but not used [-Werror=unused-function]
m68k.log:drivers/spi/spi-tegra20-slink.c:1200:12: error: 'tegra_slink_runtime_resume' defined but not used [-Werror=unused-function]
mips.log:./arch/mips/include/asm/sibyte/bcm1480_scd.h:261: error: "M_SPC_CFG_CLEAR" redefined [-Werror]
mips.log:./arch/mips/include/asm/sibyte/bcm1480_scd.h:262: error: "M_SPC_CFG_ENABLE" redefined [-Werror]
mips.log:drivers/input/joystick/analog.c:160:2: error: #warning Precise timer not defined for this architecture. [-Werror=cpp]
ppc.log:drivers/net/ethernet/cirrus/cs89x0.c:897:41: error: implicit declaration of function 'isa_virt_to_bus' [-Werror=implicit-function-declaration]
riscv32.log:drivers/gpu/drm/rockchip/cdn-dp-core.c:1126:12: error: 'cdn_dp_resume' defined but not used [-Werror=unused-function]
riscv.log:drivers/gpu/drm/rockchip/cdn-dp-core.c:1126:12: error: 'cdn_dp_resume' defined but not used [-Werror=unused-function]
s390.log:arch/s390/kernel/syscall.c:168:1: error: '__do_syscall' uses dynamic stack allocation [-Werror]
s390.log:drivers/gpu/drm/rockchip/cdn-dp-core.c:1126:12: error: 'cdn_dp_resume' defined but not used [-Werror=unused-function]
s390.log:drivers/input/joystick/analog.c:160:2: error: #warning Precise timer not defined for this architecture. [-Werror=cpp]
s390.log:drivers/spi/spi-tegra20-slink.c:1188:12: error: 'tegra_slink_runtime_suspend' defined but not used [-Werror=unused-function]
s390.log:drivers/spi/spi-tegra20-slink.c:1200:12: error: 'tegra_slink_runtime_resume' defined but not used [-Werror=unused-function]
s390.log:lib/test_kasan.c:767:1: error: 'kasan_alloca_oob_left' uses dynamic stack allocation [-Werror]
s390.log:lib/test_kasan.c:782:1: error: 'kasan_alloca_oob_right' uses dynamic stack allocation [-Werror]
s390.log:s390-linux-objcopy: error: the input file 'arch/s390/boot/compressed/syms.bin' is empty
sparc64.log:arch/sparc/kernel/mdesc.c:647:22: error: 'strcmp' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
sparc64.log:arch/sparc/kernel/mdesc.c:692:22: error: 'strcmp' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
sparc64.log:arch/sparc/kernel/mdesc.c:719:21: error: 'strcmp' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
sparc64.log:drivers/input/joystick/analog.c:160:2: error: #warning Precise timer not defined for this architecture. [-Werror=cpp]
sparc64.log:fs/qnx4/dir.c:51:32: error: 'strnlen' specified bound 48 exceeds source size 16 [-Werror=stringop-overread]
sparc64.log:./include/linux/minmax.h:20:35: error: comparison of distinct pointer types lacks a cast [-Werror]
sparc.log:crypto/blake2b_generic.c:109:1: error: the frame size of 2288 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
sparc.log:drivers/input/joystick/analog.c:160:2: error: #warning Precise timer not defined for this architecture. [-Werror=cpp]
sparc.log:drivers/spi/spi-tegra20-slink.c:1188:12: error: 'tegra_slink_runtime_suspend' defined but not used [-Werror=unused-function]
sparc.log:drivers/spi/spi-tegra20-slink.c:1200:12: error: 'tegra_slink_runtime_resume' defined but not used [-Werror=unused-function]
sparc.log:drivers/tty/serial/sunzilog.c:1128:13: error: 'sunzilog_putchar' defined but not used [-Werror=unused-function]
sparc.log:drivers/usb/host/ehci-hcd.c:1301: error: "PLATFORM_DRIVER" redefined [-Werror]
sparc.log:drivers/usb/host/ehci-sh.c:173:31: error: 'ehci_hcd_sh_driver' defined but not used [-Werror=unused-variable]
sparc.log:fs/qnx4/dir.c:51:32: error: 'strnlen' specified bound 48 exceeds source size 16 [-Werror=stringop-overread]

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Mon, Sep 6, 2021 at 4:49 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> > but I'm not seeing why that one happens on sparc64, but not on arm64
> > or x86-64. There doesn't seem to be anything architecture-specific
> > anywhere in that area.
> >
> > Funky.
>
> Not really. That is because de->di_fname is always 16 bytes but size
> can be 48 if the node is really a link. The use of de is overloaded
> in that case; de is struct qnx4_inode_entry (where di_fname is 16 bytes)
> but the actual data is struct qnx4_link_info where the name is 48 bytes
> long. A possible fix (compile tested only) is below.
>
> I think the warning/error is only reported with gcc 11.x. Do you possibly
> use an older compiler for x86/arm64 ?

No. Literally the same exact version. All of them are

    gcc version 11.2.1 20210728

from F34.

I suspect it's something about the config - a sparc64 allmodconfig
presumably doesn't end up having some of the things x86-64 has enabled
(because of different core config parameters), and then optimizes
differently as a result and shows the issue that way.

Or something. <wild handwaving>

                Linus

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/6/21 6:12 PM, Linus Torvalds wrote:
> On Mon, Sep 6, 2021 at 4:49 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>>> but I'm not seeing why that one happens on sparc64, but not on arm64
>>> or x86-64. There doesn't seem to be anything architecture-specific
>>> anywhere in that area.
>>>
>>> Funky.
>>
>> Not really. That is because de->di_fname is always 16 bytes but size
>> can be 48 if the node is really a link. The use of de is overloaded
>> in that case; de is struct qnx4_inode_entry (where di_fname is 16 bytes)
>> but the actual data is struct qnx4_link_info where the name is 48 bytes
>> long. A possible fix (compile tested only) is below.
>>
>> I think the warning/error is only reported with gcc 11.x. Do you possibly
>> use an older compiler for x86/arm64 ?
> 
> No. Literally the same exact version. All of them are
> 
>      gcc version 11.2.1 20210728
> 
> from F34.
> 
> I suspect it's something about the config - a sparc64 allmodconfig
> presumably doesn't end up having some of the things x86-64 has enabled
> (because of different core config parameters), and then optimizes
> differently as a result and shows the issue that way.
> 
> Or something. <wild handwaving>
> 

Looks like Arnd stumbled into the qnx4 problem before:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578

He might have an idea how to fix it for good.

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/6/21 7:29 PM, Guenter Roeck wrote:
> On 9/6/21 6:12 PM, Linus Torvalds wrote:
>> On Mon, Sep 6, 2021 at 4:49 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>>
>>>> but I'm not seeing why that one happens on sparc64, but not on arm64
>>>> or x86-64. There doesn't seem to be anything architecture-specific
>>>> anywhere in that area.
>>>>
>>>> Funky.
>>>
>>> Not really. That is because de->di_fname is always 16 bytes but size
>>> can be 48 if the node is really a link. The use of de is overloaded
>>> in that case; de is struct qnx4_inode_entry (where di_fname is 16 bytes)
>>> but the actual data is struct qnx4_link_info where the name is 48 bytes
>>> long. A possible fix (compile tested only) is below.
>>>
>>> I think the warning/error is only reported with gcc 11.x. Do you possibly
>>> use an older compiler for x86/arm64 ?
>>
>> No. Literally the same exact version. All of them are
>>
>>      gcc version 11.2.1 20210728
>>
>> from F34.
>>
>> I suspect it's something about the config - a sparc64 allmodconfig
>> presumably doesn't end up having some of the things x86-64 has enabled
>> (because of different core config parameters), and then optimizes
>> differently as a result and shows the issue that way.
>>
>> Or something. <wild handwaving>
>>
> 
> Looks like Arnd stumbled into the qnx4 problem before:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
> 

... and submitted a patch for it:

https://lore.kernel.org/lkml/20210322160253.4032422-6-arnd@kernel.org/

Looks like it got lost.

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Arnd Bergmann]

On Tue, Sep 7, 2021 at 1:51 AM Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, Sep 06, 2021 at 04:06:04PM -0700, Linus Torvalds wrote:
> > [ Adding some subsystem maintainers ]
> >
> > On Mon, Sep 6, 2021 at 10:06 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > >
> > > > But hopefully most cases are just "people haven't cared enough" and
> > > > easily fixed.
> > >
> > > We'll see. For my testbed I disabled the new configuration flag
> > > for the time being because its primary focus is boot tests, and
> > > there won't be any boot tests if images fail to build.
> >
> > Sure, reasonable.
> >
> > I've checked a few of the build errors by doing the appropriate cross
> > compiles, and it doesn't seem bad - but it does seem like we have a
> > number of really pointless long-standing warnings that should have
> > been fixed long ago.

I have a tree with fixes for anything that has hit on arm, arm64 or x86.
There are many reasons why some patch never made it in, but usually
it's because I was not persistent about resending the fix when the first
version didn't make it. In other cases I wasn't sure about my own fix.

> > For example, looking at sparc64, there are several build errors due to
> > those warnings now being fatal:
> >
> >  - drivers/gpu/drm/ttm/ttm_pool.c:386
> >
> >    This is a type mismatch error. It looks like __fls() on sparc64
> > returns 'int'. And the ttm_pool.c code assumes it returns 'unsigned
> > long'.
> >    Oddly enough, the very line after that line does "min_t(unsigned
> > int" to get the types in line.
> >    So  the immediate reason is "sparc64 is different".

arc is the same as sparc here, but everything else uses unsigned long.
We've come a long way in making all those helper functions consistent
in their types, but there are still a number of exceptions.

> > But the deeper
> > reason seems to be that ttm_pool.c has odd type assumptions. But that
> > warning should have been fixed long ago, either way.
> >
> >    Christian/Huang? I get the feeling that both lines in that file
> > should use the min_t(). Hmm?
> >
> >  - drivers/input/joystick/analog.c:160
> >
> >    #warning Precise timer not defined for this architecture.
> >
> >    Unfortunate. I suspect that warning just has to be removed. It has
> > never caused anything to be fixed, it's old to the point of predating
> > the git history. Dmitry?
> >
> My solution would be to just remove the old code (that isn't using ktime)
> including the module parameter that disables it. Sure, we want to be
> backward compatible, but that code is 15+ years old and should really be
> retired.

Agreed. I added a couple of architectures to the #ifdef check over time,
but realistically this driver is only ever used on x86-32 anyway, and
we don't even care about the others here.

If we remove the #else path here, I'd make it "depends on ISA ||
COMPILE_TEST".

> >  - at least a couple of stringop-overread errors. Attached is a
> > possible for for one of them.
> >
> > The stringop overread is odd, because another one of them is
> >
> >    fs/qnx4/dir.c: In function ‘qnx4_readdir’:
> >    fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds
> > source size 16 [-Werror=stringop-overread]
> >       51 |                         size = strnlen(de->di_fname, size);
> >          |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > but I'm not seeing why that one happens on sparc64, but not on arm64
> > or x86-64. There doesn't seem to be anything architecture-specific
> > anywhere in that area.
> >
> > Funky.
> >
> Not really. That is because de->di_fname is always 16 bytes but size
> can be 48 if the node is really a link. The use of de is overloaded
> in that case; de is struct qnx4_inode_entry (where di_fname is 16 bytes)
> but the actual data is struct qnx4_link_info where the name is 48 bytes
> long. A possible fix (compile tested only) is below.
>
> I think the warning/error is only reported with gcc 11.x. Do you possibly
> use an older compiler for x86/arm64 ?
>
> Anyway, below is a partial list of build errors I have seen. Some of
> them are easy to fix (such as the ones due to unused functions),
> but others seem to be tricky.

This one is worse, I think this is the same warning as the one I
reported as a false-positive in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578

I submitted a patch in
https://lore.kernel.org/all/20210322160253.4032422-6-arnd@kernel.org/

To summarize the problems:

- gcc and clang have different approaches to this type of warning: clang
  tries to only produce diagnostics that are 100% reproducible regardless
  of compiler internals, while gcc tries to use as much information as it has
  to warn about things that may go wrong, including things that it only knows
  because of inlining. Making this warning reliable is a variation of the
  halting problem, just like the -Wmaybe-uninitialized warnings. The diagnostic
  is definitely helpful and I found real bugs because of it, but you can never
  be sure that you have found all instances.

- Some of the -Wstringop-overread warnings (and related ones) from gcc are
  actually wrong, because the object size is just a heuristic. If you
have multiple
  overlapping fixed-length fields in a union, gcc-11 picks one of the union
  members to determine the size of the string buffer within the structure, even
  when the string operation uses a different union member as the output, and
  that member has the correct size.
  This is also a common problem: when a new warning option gets introduced
  first, there are false positives that get fixed in subsequent
compiler versions.


> alpha.log:arch/alpha/kernel/setup.c:493:13: error: 'strcmp' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]

I sent a couple of fixes for these: this is another false-postive bug
in gcc that made it
into the release, and it triggers on every architecture that accesses
the boot parameters
at a fixed pointer. The problem is that gcc treats '(void *)0x12345'
the same as 'NULL +
0x12345', and decides that this is an invalid NULL pointer access, so
the array has
zero readable bytes.

> alpha.log:drivers/atm/ambassador.c:1747:58: error: passing argument 1 of 'virt_to_bus' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]

Surely an alpha specific mistake, though we could fix all those
drivers to drop the
'volatile'.

> alpha.log:drivers/net/ethernet/amd/ni65.c:751:37: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]

Nobody tests ISA drivers on 64-bit architectures...

> alpha.log:drivers/net/hamradio/6pack.c:71:41: error: unsigned conversion from 'int' to 'unsigned char' changes value from '256' to '0' [-Werror=overflow]

This driver is apparently broken for any HZ >= 1024
> ppc.log:drivers/net/ethernet/cirrus/cs89x0.c:897:41: error: implicit declaration of function 'isa_virt_to_bus' [-Werror=implicit-function-declaration]

My fix is in the network tree.

> riscv32.log:drivers/gpu/drm/rockchip/cdn-dp-core.c:1126:12: error: 'cdn_dp_resume' defined but not used [-Werror=unused-function]
> riscv.log:drivers/gpu/drm/rockchip/cdn-dp-core.c:1126:12: error: 'cdn_dp_resume' defined but not used [-Werror=unused-function]

A fix was submitted today, we get at least a dozen of those for each
kernel release, and there
is a plan for avoiding them altogether, but it's a giant treewide
change that nobody has managed
to tackle.

> s390.log:arch/s390/kernel/syscall.c:168:1: error: '__do_syscall' uses dynamic stack allocation [-Werror]

This is add_random_kstack_offset(). No idea why it doesn't trigger on
x86, but that
warning should probably get shut up inside of the macro.

> sparc64.log:arch/sparc/kernel/mdesc.c:647:22: error: 'strcmp' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
> sparc64.log:arch/sparc/kernel/mdesc.c:692:22: error: 'strcmp' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
> sparc64.log:arch/sparc/kernel/mdesc.c:719:21: error: 'strcmp' reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]

Same as on alpha

       Arnd

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On Mon, Sep 06, 2021 at 04:49:21PM -0700, Guenter Roeck wrote:
> On Mon, Sep 06, 2021 at 04:06:04PM -0700, Linus Torvalds wrote:
[ ... ]

> >  - at least a couple of stringop-overread errors. Attached is a
> > possible for for one of them.
> > 

I keep seeing problems like this.

drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
./arch/m68k/include/asm/string.h:72:25: error: '__builtin_memcpy' reading 6 bytes from a region of size 0 [-Werror=stringop-overread]
   72 | #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/i825xx/82596.c:1147:17: note: in expansion of macro 'memcpy'
 1147 |                 memcpy(eth_addr, (void *) 0xfffc1f2c, ETH_ALEN);        /* YUCK! Get addr from NOVRAM */
      |                 ^~~~~~
cc1: all warnings being treated as errors

It is seen with gcc 11.x whenever a memXXX or strXXX function parameter
is a pointer to a fixed address. gcc is happy if "(void *) 0xfffc1f2c"
is passed to a global function which does nothing but return the address,
such as:

void *sanitize_address(void *address)
{
	return address;
}

and:

	memcpy(eth_addr, sanitize_address((void *) 0xfffc1f2c), ETH_ALEN);

but that just seems weird. Is there a better solution ?

Thanks,
Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Al Viro]

On Tue, Sep 07, 2021 at 09:28:38PM -0700, Guenter Roeck wrote:

> 	memcpy(eth_addr, sanitize_address((void *) 0xfffc1f2c), ETH_ALEN);
> 
> but that just seems weird. Is there a better solution ?

(char (*)[ETH_ALEN])?  Said that, shouldn't that be doing something like
ioremap(), rather than casting explicit constants?

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/7/21 9:48 PM, Al Viro wrote:
> On Tue, Sep 07, 2021 at 09:28:38PM -0700, Guenter Roeck wrote:
> 
>> 	memcpy(eth_addr, sanitize_address((void *) 0xfffc1f2c), ETH_ALEN);
>>
>> but that just seems weird. Is there a better solution ?
> 
> (char (*)[ETH_ALEN])?  Said that, shouldn't that be doing something like
> ioremap(), rather than casting explicit constants?
> 

Typecasts or even assigning the address to a variable does not help.
The sanitizer function can not be static either.

I don't know the hardware, so I can not answer the ioremap() question.

This is just one example, though; there are several sprinkled throughout
the code. Another is:

arch/parisc/kernel/setup.c:	running_on_qemu = (memcmp(&PAGE0->pad0, "SeaBIOS", 8) == 0);

where

#define PAGE0   ((struct zeropage *)__PAGE_OFFSET)

and __PAGE_OFFSET depends on the configuration.

That code runs early in setup; I don't think ioremap() would even
be available at that time. A workaround for that problem would be
a global variable pointing to PAGE0 (or of course an address sanitizer
function), but again that seems odd just to make the compiler happy.

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Geert Uytterhoeven]

On Wed, Sep 8, 2021 at 7:16 AM Guenter Roeck <linux@roeck-us.net> wrote:
> On 9/7/21 9:48 PM, Al Viro wrote:
> > On Tue, Sep 07, 2021 at 09:28:38PM -0700, Guenter Roeck wrote:
> >>      memcpy(eth_addr, sanitize_address((void *) 0xfffc1f2c), ETH_ALEN);
> >>
> >> but that just seems weird. Is there a better solution ?
> >
> > (char (*)[ETH_ALEN])?  Said that, shouldn't that be doing something like
> > ioremap(), rather than casting explicit constants?
>
> Typecasts or even assigning the address to a variable does not help.
> The sanitizer function can not be static either.

So it can only be fixed by obfuscating the constant address in a
chain of out-of-line functions...
How is this compiler to be used for bare-metal programming?

> I don't know the hardware, so I can not answer the ioremap() question.

Yes it should.  But this driver dates back to 2.1.110, when only
half of the architectures already had ioremap().

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Arnd Bergmann]

On Wed, Sep 8, 2021 at 9:49 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Wed, Sep 8, 2021 at 7:16 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > On 9/7/21 9:48 PM, Al Viro wrote:
> > > On Tue, Sep 07, 2021 at 09:28:38PM -0700, Guenter Roeck wrote:
> > >>      memcpy(eth_addr, sanitize_address((void *) 0xfffc1f2c), ETH_ALEN);
> > >>
> > >> but that just seems weird. Is there a better solution ?
> > >
> > > (char (*)[ETH_ALEN])?  Said that, shouldn't that be doing something like
> > > ioremap(), rather than casting explicit constants?
> >
> > Typecasts or even assigning the address to a variable does not help.
> > The sanitizer function can not be static either.
>
> So it can only be fixed by obfuscating the constant address in a
> chain of out-of-line functions...
> How is this compiler to be used for bare-metal programming?

I reported this as a gcc bug when I first saw it back in March:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578

Martin Sebor suggested marking the pointer as 'volatile' as a workaround,
which is probably fine for bare-metal programming, but I would consider
that bad style for the kernel boot arguments. The RELOC_HIDE trick is probably
fine here, as there are only a couple of instances, and for the network
driver, using volatile is probably appropriate as well.

I still hope this can be fixed in a future gcc-11.x release. Maybe we should
add further instances of the problem on the gcc bug to boost the priority?

> > I don't know the hardware, so I can not answer the ioremap() question.
>
> Yes it should.  But this driver dates back to 2.1.110, when only
> half of the architectures already had ioremap().

How does  mvme16x even create the mapping? Is this a virtual address
that is hardwired to the bus or do you have a static mapping somewhere?
I see two other drivers accessing the nvram here

arch/m68k/mvme16x/config.c:static MK48T08ptr_t volatile rtc =
(MK48T08ptr_t)MVME_RTC_BASE;
arch/m68k/mvme16x/rtc.c:        volatile MK48T08ptr_t rtc =
(MK48T08ptr_t)MVME_RTC_BASE;

The same trick should work here, just create a local variable with a
volatile pointer and read from that.

       Arnd

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Geert Uytterhoeven]

Hi Arnd,

On Wed, Sep 8, 2021 at 11:50 AM Arnd Bergmann <arnd@kernel.org> wrote:
> On Wed, Sep 8, 2021 at 9:49 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, Sep 8, 2021 at 7:16 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > > On 9/7/21 9:48 PM, Al Viro wrote:
> > > > On Tue, Sep 07, 2021 at 09:28:38PM -0700, Guenter Roeck wrote:
> > > >>      memcpy(eth_addr, sanitize_address((void *) 0xfffc1f2c), ETH_ALEN);
> > > >>
> > > >> but that just seems weird. Is there a better solution ?
> > > >
> > > > (char (*)[ETH_ALEN])?  Said that, shouldn't that be doing something like
> > > > ioremap(), rather than casting explicit constants?
> > >
> > > Typecasts or even assigning the address to a variable does not help.
> > > The sanitizer function can not be static either.
> >
> > So it can only be fixed by obfuscating the constant address in a
> > chain of out-of-line functions...
> > How is this compiler to be used for bare-metal programming?
>
> I reported this as a gcc bug when I first saw it back in March:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
>
> Martin Sebor suggested marking the pointer as 'volatile' as a workaround,
> which is probably fine for bare-metal programming, but I would consider
> that bad style for the kernel boot arguments. The RELOC_HIDE trick is probably
> fine here, as there are only a couple of instances, and for the network
> driver, using volatile is probably appropriate as well.

Yeah, volatile should be fine for drivers.
In fact this is one of the few places where I/O registers are accessed
without involving volatile.

> I still hope this can be fixed in a future gcc-11.x release. Maybe we should
> add further instances of the problem on the gcc bug to boost the priority?
>
> > > I don't know the hardware, so I can not answer the ioremap() question.
> >
> > Yes it should.  But this driver dates back to 2.1.110, when only
> > half of the architectures already had ioremap().
>
> How does  mvme16x even create the mapping? Is this a virtual address
> that is hardwired to the bus or do you have a static mapping somewhere?

It's part of the transparent mapping of the top address space for
I/O devices in arch/m68k/kernel/head.S.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Geert Uytterhoeven]

On Wed, Sep 8, 2021 at 11:50 AM Arnd Bergmann <arnd@kernel.org> wrote:
> On Wed, Sep 8, 2021 at 9:49 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Wed, Sep 8, 2021 at 7:16 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > > On 9/7/21 9:48 PM, Al Viro wrote:
> > > > On Tue, Sep 07, 2021 at 09:28:38PM -0700, Guenter Roeck wrote:
> > > >>      memcpy(eth_addr, sanitize_address((void *) 0xfffc1f2c), ETH_ALEN);
> > > >>
> > > >> but that just seems weird. Is there a better solution ?
> > > >
> > > > (char (*)[ETH_ALEN])?  Said that, shouldn't that be doing something like
> > > > ioremap(), rather than casting explicit constants?
> > >
> > > Typecasts or even assigning the address to a variable does not help.
> > > The sanitizer function can not be static either.
> >
> > So it can only be fixed by obfuscating the constant address in a
> > chain of out-of-line functions...
> > How is this compiler to be used for bare-metal programming?
>
> I reported this as a gcc bug when I first saw it back in March:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
>
> Martin Sebor suggested marking the pointer as 'volatile' as a workaround,
> which is probably fine for bare-metal programming, but I would consider
> that bad style for the kernel boot arguments. The RELOC_HIDE trick is probably
> fine here, as there are only a couple of instances, and for the network
> driver, using volatile is probably appropriate as well.

A related one, I guess, is:

    arch/m68k/include/asm/string.h:72:25: error: argument 2 null where
non-null expected [-Werror=nonnull]
       72 | #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
    drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:387:4:
note: in expansion of macro ‘memcpy’
      387 |    memcpy((char *)kmap(pages[0]) +
          |    ^~~~~~

Seen with my sun3-allmodconfig build.
As NO_DMA=y, dmam_alloc_coherent() returns NULL, and the compiler
discovers that g_fragments_base is never assigned to and thus must
be NULL.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/8/21 2:50 AM, Arnd Bergmann wrote:
> On Wed, Sep 8, 2021 at 9:49 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Wed, Sep 8, 2021 at 7:16 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>> On 9/7/21 9:48 PM, Al Viro wrote:
>>>> On Tue, Sep 07, 2021 at 09:28:38PM -0700, Guenter Roeck wrote:
>>>>>       memcpy(eth_addr, sanitize_address((void *) 0xfffc1f2c), ETH_ALEN);
>>>>>
>>>>> but that just seems weird. Is there a better solution ?
>>>>
>>>> (char (*)[ETH_ALEN])?  Said that, shouldn't that be doing something like
>>>> ioremap(), rather than casting explicit constants?
>>>
>>> Typecasts or even assigning the address to a variable does not help.
>>> The sanitizer function can not be static either.
>>
>> So it can only be fixed by obfuscating the constant address in a
>> chain of out-of-line functions...
>> How is this compiler to be used for bare-metal programming?
> 
> I reported this as a gcc bug when I first saw it back in March:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
> 
> Martin Sebor suggested marking the pointer as 'volatile' as a workaround,
> which is probably fine for bare-metal programming, but I would consider
> that bad style for the kernel boot arguments. The RELOC_HIDE trick is probably
> fine here, as there are only a couple of instances, and for the network
> driver, using volatile is probably appropriate as well.
> 
> I still hope this can be fixed in a future gcc-11.x release. Maybe we should
> add further instances of the problem on the gcc bug to boost the priority?
> 
>>> I don't know the hardware, so I can not answer the ioremap() question.
>>
>> Yes it should.  But this driver dates back to 2.1.110, when only
>> half of the architectures already had ioremap().
> 
> How does  mvme16x even create the mapping? Is this a virtual address
> that is hardwired to the bus or do you have a static mapping somewhere?
> I see two other drivers accessing the nvram here
> 
> arch/m68k/mvme16x/config.c:static MK48T08ptr_t volatile rtc =
> (MK48T08ptr_t)MVME_RTC_BASE;

Is that even correct ? I am always shaky with qualifiers, but doesn't
that mean that the pointer is volatile, not the object it points to ?

> arch/m68k/mvme16x/rtc.c:        volatile MK48T08ptr_t rtc =
> (MK48T08ptr_t)MVME_RTC_BASE;
> 
> The same trick should work here, just create a local variable with a
> volatile pointer and read from that.
> 

I had tried that; it doesn't work because then the compiler complains
that the 'volatile' qualifier is discarded when passing the argument.

drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
drivers/net/ethernet/i825xx/82596.c:1147:34: error:
	passing argument 2 of '__builtin_memcpy' discards 'volatile' qualifier from pointer target type

Oddly enough, a memcpy on the 'rtc' variable doesn't fail,
neither with nor without volatile. Something else is going on.

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Al Viro]

On Wed, Sep 08, 2021 at 05:42:30AM -0700, Guenter Roeck wrote:

> Oddly enough, a memcpy on the 'rtc' variable doesn't fail,
> neither with nor without volatile. Something else is going on.

While we are at it, would memcpy_fromio() complain?  Seeing that
this is what's really intended there...

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/8/21 6:19 AM, Al Viro wrote:
> On Wed, Sep 08, 2021 at 05:42:30AM -0700, Guenter Roeck wrote:
> 
>> Oddly enough, a memcpy on the 'rtc' variable doesn't fail,
>> neither with nor without volatile. Something else is going on.
> 
> While we are at it, would memcpy_fromio() complain?  Seeing that
> this is what's really intended there...
> 

It doesn't make a difference on m68k.

#define memcpy_fromio memcpy_fromio
static inline void memcpy_fromio(void *dst, const volatile void __iomem *src,
                                  int count)
{
         __builtin_memcpy(dst, (void __force *) src, count);
}

It boils down to the use of __builtin_memcpy(). m68k implements its own version
of memcpy(). If that is used, everything works fine. However, if a file includes
<linux/string.h>, memcpy is replaced with __builtin_memcpy:

#define __HAVE_ARCH_MEMCPY
extern void *memcpy(void *, const void *, __kernel_size_t);
#define memcpy(d, s, n) __builtin_memcpy(d, s, n)

and the compilation fails.

That also explains why only some architectures/files are affected.
Presumably those are the architectures which use __builtin_memcpy().

Guenter

RE: [PATCH] Enable '-Werror' by default for all kernel builds

[David Laight]

From: Arnd Bergmann
> Sent: 08 September 2021 10:50
...
> > > I don't know the hardware, so I can not answer the ioremap() question.
> >
> > Yes it should.  But this driver dates back to 2.1.110, when only
> > half of the architectures already had ioremap().
> 
> How does  mvme16x even create the mapping? Is this a virtual address
> that is hardwired to the bus or do you have a static mapping somewhere?
> I see two other drivers accessing the nvram here
> 
> arch/m68k/mvme16x/config.c:static MK48T08ptr_t volatile rtc = (MK48T08ptr_t)MVME_RTC_BASE;
> arch/m68k/mvme16x/rtc.c:        volatile MK48T08ptr_t rtc = (MK48T08ptr_t)MVME_RTC_BASE;
> 
> The same trick should work here, just create a local variable with a
> volatile pointer and read from that.

Or define a C 'extern' for the actual data and get the linker script
to assign a fixed value to the symbol.
(Although that does pollute the global namespace.)

An alternative is to use an asm statement so the compiler
cannot track the actual value.
Something like:

#define launder(x) asm volatile( "" : "+r" (x))

	MK48T08ptr_t rtc = (void *)MVME_RTC_BASE;
	launder(rtc);

That also works a bit like READ_ONCE() except that is works
on a value that is (hopefully) already in a register rather
that during the read from memory.
Useful when the compiler's 'value tracking' pessimises code.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Tue, Sep 7, 2021 at 9:28 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> It is seen with gcc 11.x whenever a memXXX or strXXX function parameter
> is a pointer to a fixed address.

I wonder why I don't see it with gcc 11.2 here on x86-64.

> gcc is happy if "(void *) 0xfffc1f2c"
> is passed to a global function which does nothing but return the address,
> such as:
>
> void *sanitize_address(void *address)
> {
>         return address;
> }

We have had reasons to do things like that before for somewhat similar
(well, opposite) reasons - trying to disassociate some pointer from
its originating symbol type.

Look at RELOC_HIDE().

It might be worth it having something similar for "absolute_pointer()".

Entirely untested "written-in-the-MUA" garbage:

   #define absolute_pointer(val) \
        ({ void *__res; __asm__("":"=r" (__res):"0" ((unsigned
long)(val))); __res; })

I dunno.

               Linus

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/7/21 9:55 PM, Linus Torvalds wrote:
> On Tue, Sep 7, 2021 at 9:28 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> It is seen with gcc 11.x whenever a memXXX or strXXX function parameter
>> is a pointer to a fixed address.
> 
> I wonder why I don't see it with gcc 11.2 here on x86-64.
> 

I see the problem only on some architectures. No idea what triggers it,
but it is definitely architecture dependent.

>> gcc is happy if "(void *) 0xfffc1f2c"
>> is passed to a global function which does nothing but return the address,
>> such as:
>>
>> void *sanitize_address(void *address)
>> {
>>          return address;
>> }
> 
> We have had reasons to do things like that before for somewhat similar
> (well, opposite) reasons - trying to disassociate some pointer from
> its originating symbol type.
> 
> Look at RELOC_HIDE().
> 
> It might be worth it having something similar for "absolute_pointer()".
> 
> Entirely untested "written-in-the-MUA" garbage:
> 
>     #define absolute_pointer(val) \
>          ({ void *__res; __asm__("":"=r" (__res):"0" ((unsigned
> long)(val))); __res; })
> 

or:

#define absolute_pointer(val) RELOC_HIDE(val, 0)

or maybe:

#define absolute_pointer(val) RELOC_HIDE((void *)val, 0)

would do the same (though the first variant needs a pointer as argument).
All of those compile.

I tested the first and the last option on qemu:parisc and confirmed that
both work as expected.

I'd be happy to send a formal patch. Which one do you prefer, and where
should I put it ?

Thanks,
Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Huang Rui]

On Tue, Sep 07, 2021 at 07:06:04AM +0800, Linus Torvalds wrote:
> [ Adding some subsystem maintainers ]
> 
> On Mon, Sep 6, 2021 at 10:06 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > > But hopefully most cases are just "people haven't cared enough" and
> > > easily fixed.
> >
> > We'll see. For my testbed I disabled the new configuration flag
> > for the time being because its primary focus is boot tests, and
> > there won't be any boot tests if images fail to build.
> 
> Sure, reasonable.
> 
> I've checked a few of the build errors by doing the appropriate cross
> compiles, and it doesn't seem bad - but it does seem like we have a
> number of really pointless long-standing warnings that should have
> been fixed long ago.
> 
> For example, looking at sparc64, there are several build errors due to
> those warnings now being fatal:
> 
>  - drivers/gpu/drm/ttm/ttm_pool.c:386
> 
>    This is a type mismatch error. It looks like __fls() on sparc64
> returns 'int'. And the ttm_pool.c code assumes it returns 'unsigned
> long'.
> 
>    Oddly enough, the very line after that line does "min_t(unsigned
> int" to get the types in line.
> 
>    So  the immediate reason is "sparc64 is different". But the deeper
> reason seems to be that ttm_pool.c has odd type assumptions. But that
> warning should have been fixed long ago, either way.
> 
>    Christian/Huang? I get the feeling that both lines in that file
> should use the min_t(). Hmm?


Shall we align the return type like __fls() on all the arches?

But it looks OK for me to change min to min_t() here as well, I can file a
patch to the update:

-       for (order = min(MAX_ORDER - 1UL, __fls(num_pages)); num_pages;
+       for (order = min_t(unsigned int, MAX_ORDER - 1UL, __fls(num_pages)); num_pages;

Christian, what's your opinion?

Thanks,
Ray

> 
>  - drivers/input/joystick/analog.c:160
> 
>    #warning Precise timer not defined for this architecture.
> 
>    Unfortunate. I suspect that warning just has to be removed. It has
> never caused anything to be fixed, it's old to the point of predating
> the git history. Dmitry?
> 
>  - at least a couple of stringop-overread errors. Attached is a
> possible for for one of them.
> 
> The stringop overread is odd, because another one of them is
> 
>    fs/qnx4/dir.c: In function ‘qnx4_readdir’:
>    fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds
> source size 16 [-Werror=stringop-overread]
>       51 |                         size = strnlen(de->di_fname, size);
>          |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> but I'm not seeing why that one happens on sparc64, but not on arm64
> or x86-64. There doesn't seem to be anything architecture-specific
> anywhere in that area.
> 
> Funky.
> 
> Davem - attached patch compiles cleanly for me, but I'm not sure it's
> necessarily the right thing to do, and I didn't check the code
> generation. Maybe it screws up. Can somebody test on sparc64 and
> perhaps think about it more than I did?
> 
>                Linus

>  arch/sparc/kernel/mdesc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c
> index 8e645ddac58e..30f171b7b00c 100644
> --- a/arch/sparc/kernel/mdesc.c
> +++ b/arch/sparc/kernel/mdesc.c
> @@ -39,6 +39,7 @@ struct mdesc_hdr {
>  	u32	node_sz; /* node block size */
>  	u32	name_sz; /* name block size */
>  	u32	data_sz; /* data block size */
> +	char	data[];
>  } __attribute__((aligned(16)));
>  
>  struct mdesc_elem {
> @@ -612,7 +613,7 @@ EXPORT_SYMBOL(mdesc_get_node_info);
>  
>  static struct mdesc_elem *node_block(struct mdesc_hdr *mdesc)
>  {
> -	return (struct mdesc_elem *) (mdesc + 1);
> +	return (struct mdesc_elem *) mdesc->data;
>  }
>  
>  static void *name_block(struct mdesc_hdr *mdesc)

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Christian König]

Am 07.09.21 um 07:32 schrieb Huang Rui:
> On Tue, Sep 07, 2021 at 07:06:04AM +0800, Linus Torvalds wrote:
>> [ Adding some subsystem maintainers ]
>>
>> On Mon, Sep 6, 2021 at 10:06 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>>> But hopefully most cases are just "people haven't cared enough" and
>>>> easily fixed.
>>> We'll see. For my testbed I disabled the new configuration flag
>>> for the time being because its primary focus is boot tests, and
>>> there won't be any boot tests if images fail to build.
>> Sure, reasonable.
>>
>> I've checked a few of the build errors by doing the appropriate cross
>> compiles, and it doesn't seem bad - but it does seem like we have a
>> number of really pointless long-standing warnings that should have
>> been fixed long ago.
>>
>> For example, looking at sparc64, there are several build errors due to
>> those warnings now being fatal:
>>
>>   - drivers/gpu/drm/ttm/ttm_pool.c:386
>>
>>     This is a type mismatch error. It looks like __fls() on sparc64
>> returns 'int'. And the ttm_pool.c code assumes it returns 'unsigned
>> long'.
>>
>>     Oddly enough, the very line after that line does "min_t(unsigned
>> int" to get the types in line.
>>
>>     So  the immediate reason is "sparc64 is different". But the deeper
>> reason seems to be that ttm_pool.c has odd type assumptions. But that
>> warning should have been fixed long ago, either way.
>>
>>     Christian/Huang? I get the feeling that both lines in that file
>> should use the min_t(). Hmm?
>
> Shall we align the return type like __fls() on all the arches?

I think so, yes. IIRC I was a bit surprised that it returns UL on x86. I 
mean the maximum possible value here is 63.

>
> But it looks OK for me to change min to min_t() here as well, I can file a
> patch to the update:
>
> -       for (order = min(MAX_ORDER - 1UL, __fls(num_pages)); num_pages;
> +       for (order = min_t(unsigned int, MAX_ORDER - 1UL, __fls(num_pages)); num_pages;
>
> Christian, what's your opinion?

The "MAX_ORDER - 1UL" can now be changed to "MAX_ORDER - 1", but apart 
from that looks good to me.

Thanks,
Christian.

>
> Thanks,
> Ray
>
>>   - drivers/input/joystick/analog.c:160
>>
>>     #warning Precise timer not defined for this architecture.
>>
>>     Unfortunate. I suspect that warning just has to be removed. It has
>> never caused anything to be fixed, it's old to the point of predating
>> the git history. Dmitry?
>>
>>   - at least a couple of stringop-overread errors. Attached is a
>> possible for for one of them.
>>
>> The stringop overread is odd, because another one of them is
>>
>>     fs/qnx4/dir.c: In function ‘qnx4_readdir’:
>>     fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds
>> source size 16 [-Werror=stringop-overread]
>>        51 |                         size = strnlen(de->di_fname, size);
>>           |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> but I'm not seeing why that one happens on sparc64, but not on arm64
>> or x86-64. There doesn't seem to be anything architecture-specific
>> anywhere in that area.
>>
>> Funky.
>>
>> Davem - attached patch compiles cleanly for me, but I'm not sure it's
>> necessarily the right thing to do, and I didn't check the code
>> generation. Maybe it screws up. Can somebody test on sparc64 and
>> perhaps think about it more than I did?
>>
>>                 Linus
>>   arch/sparc/kernel/mdesc.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c
>> index 8e645ddac58e..30f171b7b00c 100644
>> --- a/arch/sparc/kernel/mdesc.c
>> +++ b/arch/sparc/kernel/mdesc.c
>> @@ -39,6 +39,7 @@ struct mdesc_hdr {
>>   	u32	node_sz; /* node block size */
>>   	u32	name_sz; /* name block size */
>>   	u32	data_sz; /* data block size */
>> +	char	data[];
>>   } __attribute__((aligned(16)));
>>   
>>   struct mdesc_elem {
>> @@ -612,7 +613,7 @@ EXPORT_SYMBOL(mdesc_get_node_info);
>>   
>>   static struct mdesc_elem *node_block(struct mdesc_hdr *mdesc)
>>   {
>> -	return (struct mdesc_elem *) (mdesc + 1);
>> +	return (struct mdesc_elem *) mdesc->data;
>>   }
>>   
>>   static void *name_block(struct mdesc_hdr *mdesc)

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Geert Uytterhoeven]

On Tue, Sep 7, 2021 at 8:25 AM Christian König <christian.koenig@amd.com> wrote:
> Am 07.09.21 um 07:32 schrieb Huang Rui:
> > On Tue, Sep 07, 2021 at 07:06:04AM +0800, Linus Torvalds wrote:
> >> [ Adding some subsystem maintainers ]
> >>
> >> On Mon, Sep 6, 2021 at 10:06 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >>>> But hopefully most cases are just "people haven't cared enough" and
> >>>> easily fixed.
> >>> We'll see. For my testbed I disabled the new configuration flag
> >>> for the time being because its primary focus is boot tests, and
> >>> there won't be any boot tests if images fail to build.
> >> Sure, reasonable.
> >>
> >> I've checked a few of the build errors by doing the appropriate cross
> >> compiles, and it doesn't seem bad - but it does seem like we have a
> >> number of really pointless long-standing warnings that should have
> >> been fixed long ago.
> >>
> >> For example, looking at sparc64, there are several build errors due to
> >> those warnings now being fatal:
> >>
> >>   - drivers/gpu/drm/ttm/ttm_pool.c:386
> >>
> >>     This is a type mismatch error. It looks like __fls() on sparc64
> >> returns 'int'. And the ttm_pool.c code assumes it returns 'unsigned
> >> long'.
> >>
> >>     Oddly enough, the very line after that line does "min_t(unsigned
> >> int" to get the types in line.
> >>
> >>     So  the immediate reason is "sparc64 is different". But the deeper
> >> reason seems to be that ttm_pool.c has odd type assumptions. But that
> >> warning should have been fixed long ago, either way.
> >>
> >>     Christian/Huang? I get the feeling that both lines in that file
> >> should use the min_t(). Hmm?
> >
> > Shall we align the return type like __fls() on all the arches?
>
> I think so, yes. IIRC I was a bit surprised that it returns UL on x86. I
> mean the maximum possible value here is 63.

And ffs() returns int, like in ffs(3).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Nathan Chancellor]

On Mon, Sep 06, 2021 at 09:12:12AM -0700, Linus Torvalds wrote:
> On Mon, Sep 6, 2021 at 7:26 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > Build results:
> >         total: 153 pass: 89 fail: 64
> 
> Well, that sadly proves the point of that patch. x86-64 may be clean,
> because I have required it manually. Others not necessarily so much..
> 
> I've got at least one sparc64 fix in my inbox. It _might_ fix some
> other cases too (syscall checking), but I suspect it's one of those
> "death by a thousand cuts" situations, not just one or two issues that
> show up.
> 
> Do you end up exposing the errors anywhere where I can take a look?
> 
> If some of them are just because of bad tooling on certain
> architectures (ie fundamentally "this is unfixable, because we use
> gcc-XYZ that just always causes warnings") then those we could/should
> just disable -Werror for those and forget about them.
> 
> But hopefully most cases are just "people haven't cared enough" and
> easily fixed.

Our clang builds got bit pretty hard by this. From my local builds
(clang-14), the following ones failed (file name describes the config)
along with the errors plus some triage. -Wframe-larger-than= appears to
be the most common warning. I apologize if this email is too long or
convoluted, I can try to break it down better in the future.



arm32-allmodconfig.log: arch/arm/lib/xor-neon.c:30:2: error: This code requires at least version 4.6 of GCC [-Werror,-W#warnings]
arm32-alpine.log: arch/arm/lib/xor-neon.c:30:2: error: This code requires at least version 4.6 of GCC [-Werror,-W#warnings]
arm32-debian.log: arch/arm/lib/xor-neon.c:30:2: error: This code requires at least version 4.6 of GCC [-Werror,-W#warnings
arm32-fedora.log: arch/arm/lib/xor-neon.c:30:2: error: This code requires at least version 4.6 of GCC [-Werror,-W#warnings]
arm32-opensuse.log: arch/arm/lib/xor-neon.c:30:2: error: This code requires at least version 4.6 of GCC [-Werror,-W#warnings]
arm32-v7-archlinux.log:arch/arm/lib/xor-neon.c:30:2: error: This code requires at least version 4.6 of GCC [-Werror,-W#warnings]

This has been tracked for a while with no real resolution:

https://github.com/ClangBuiltLinux/linux/issues/496
https://github.com/ClangBuiltLinux/linux/issues/503
https://lore.kernel.org/r/20190528235742.105510-1-natechancellor@gmail.com/
https://lore.kernel.org/r/20201106051436.2384842-1-adrian.ratiu@collabora.com/
https://lore.kernel.org/r/20201112212457.2042105-1-adrian.ratiu@collabora.com/



arm32-allmodconfig.log: drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c:157:11: error: variable 'err' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
arm32-allmodconfig.log: drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c:257:7: error: variable 'err' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
arm32-allmodconfig.log: drivers/net/ethernet/mellanox/mlx5/core/en/rep/bridge.c:262:7: error: variable 'err' is used uninitialized whenever switch case is taken [-Werror,-Wsometimes-uninitialized]

This affected a great number of configs. It is known and is being fixed:

https://lore.kernel.org/r/CA+G9fYsV7sTfaefGj3bpkvVdRQUeiWCVRiu6ovjtM=qri-HJ8g@mail.gmail.com/
https://lore.kernel.org/r/20210902190554.211497-4-saeed@kernel.org/

Unfortunately, these uninitialized warnings will constantly plague us
because GCC does not warn due to -Wmaybe-uninitialized being disabled
because it is not as reliable as the clang warning.



arm32-allmodconfig.log: crypto/wp512.c:782:13: error: stack frame size (1176) exceeds limit (1024) in function 'wp512_process_buffer' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/firmware/tegra/bpmp-debugfs.c:294:12: error: stack frame size (1256) exceeds limit (1024) in function 'bpmp_debug_show' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/firmware/tegra/bpmp-debugfs.c:357:16: error: stack frame size (1264) exceeds limit (1024) in function 'bpmp_debug_store' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1384) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5560) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/mtd/chips/cfi_cmdset_0001.c:1872:12: error: stack frame size (1064) exceeds limit (1024) in function 'cfi_intelext_writev' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/ntb/hw/idt/ntb_hw_idt.c:1041:27: error: stack frame size (1032) exceeds limit (1024) in function 'idt_scan_mws' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/staging/fbtft/fbtft-core.c:902:12: error: stack frame size (1072) exceeds limit (1024) in function 'fbtft_init_display_from_property' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/staging/fbtft/fbtft-core.c:992:5: error: stack frame size (1064) exceeds limit (1024) in function 'fbtft_init_display' [-Werror,-Wframe-larger-than]
arm32-allmodconfig.log: drivers/staging/rtl8723bs/core/rtw_security.c:1288:5: error: stack frame size (1040) exceeds limit (1024) in function 'rtw_aes_decrypt' [-Werror,-Wframe-larger-than]
arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1376) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5384) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]

Aside from the dce_calcs.c warnings, these do not seem too bad. I
believe allmodconfig turns on UBSAN but it could also be aggressive
inlining by clang. I intend to look at all -Wframe-large-than warnings
closely later.



arm64-alpine.log:drivers/scsi/lpfc/lpfc_init.c:11608:48: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
arm64-alpine.log:drivers/scsi/lpfc/lpfc_init.c:8280:29: error: no member named 'c_stat' in 'struct lpfc_sli4_hba'
arm64-alpine.log:drivers/scsi/lpfc/lpfc_init.c:9092:48: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
arm64-alpine.log:drivers/scsi/lpfc/lpfc_nvme.c:1592:3: error: use of undeclared identifier 'start'
arm64-alpine.log:drivers/scsi/lpfc/lpfc_nvme.c:1639:28: error: use of undeclared identifier 'start'; did you mean 'cstat'?
arm64-alpine.log:drivers/scsi/lpfc/lpfc_scsi.c:5587:2: error: use of undeclared identifier 'start'
arm64-alpine.log:drivers/scsi/lpfc/lpfc_scsi.c:5670:27: error: use of undeclared identifier 'start'
x86_64-alpine.log:drivers/scsi/lpfc/lpfc_init.c:11608:48: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
x86_64-alpine.log:drivers/scsi/lpfc/lpfc_init.c:8280:29: error: no member named 'c_stat' in 'struct lpfc_sli4_hba'
x86_64-alpine.log:drivers/scsi/lpfc/lpfc_init.c:9092:48: error: shift count >= width of type [-Werror,-Wshift-count-overflow]
x86_64-alpine.log:drivers/scsi/lpfc/lpfc_nvme.c:1592:3: error: use of undeclared identifier 'start'
x86_64-alpine.log:drivers/scsi/lpfc/lpfc_nvme.c:1639:28: error: use of undeclared identifier 'start'
x86_64-alpine.log:drivers/scsi/lpfc/lpfc_scsi.c:5587:2: error: use of undeclared identifier 'start'
x86_64-alpine.log:drivers/scsi/lpfc/lpfc_scsi.c:5670:27: error: use of undeclared identifier 'start'; did you mean 'stac'?

The -Wshift-count-overflow warnings only show because there are other
errors in this file. This appears to be because some variables or
members of a structure are defined when the lpfc debug config is
disabled. I'll send patches for these later.



arm64-archlinux.log: arch/arm64/crypto/aes-neonbs-glue.c:270:12: error: stack frame size (1056) exceeds limit (1024) in function 'aesbs_xts_setkey' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/char/ipmi/ipmi_msghandler.c:4850:13: error: stack frame size (1072) exceeds limit (1024) in function 'ipmi_panic_request_and_wait' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/crypto/ccp/ccp-ops.c:629:1: error: stack frame size (1072) exceeds limit (1024) in function 'ccp_run_aes_gcm_cmd' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/firmware/tegra/bpmp-debugfs.c:294:12: error: stack frame size (1296) exceeds limit (1024) in function 'bpmp_debug_show' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/firmware/tegra/bpmp-debugfs.c:357:16: error: stack frame size (1328) exceeds limit (1024) in function 'bpmp_debug_store' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/gpu/drm/radeon/radeon_cs.c:661:5: error: stack frame size (1184) exceeds limit (1024) in function 'radeon_cs_ioctl' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1779:5: error: stack frame size (1152) exceeds limit (1024) in function 'arm_smmu_atc_inv_domain' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1851:13: error: stack frame size (1136) exceeds limit (1024) in function '__arm_smmu_tlb_inv_range' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:2295:13: error: stack frame size (1136) exceeds limit (1024) in function 'arm_smmu_disable_ats' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:952:13: error: stack frame size (1152) exceeds limit (1024) in function 'arm_smmu_sync_cd' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/net/wireguard/allowedips.c:255:6: error: stack frame size (1120) exceeds limit (1024) in function 'wg_allowedips_free' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/net/wireguard/allowedips.c:53:13: error: stack frame size (1088) exceeds limit (1024) in function 'root_free_rcu' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/staging/fbtft/fb_hx8353d.c:20:12: error: stack frame size (1040) exceeds limit (1024) in function 'init_display' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/staging/fbtft/fb_ssd1331.c:131:12: error: stack frame size (1360) exceeds limit (1024) in function 'set_gamma' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/staging/fbtft/fb_ssd1351.c:120:12: error: stack frame size (1360) exceeds limit (1024) in function 'set_gamma' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/staging/fbtft/fbtft-core.c:902:12: error: stack frame size (1040) exceeds limit (1024) in function 'fbtft_init_display_from_property' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/vhost/scsi.c:1543:1: error: stack frame size (1152) exceeds limit (1024) in function 'vhost_scsi_set_endpoint' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/vhost/scsi.c:1670:1: error: stack frame size (1136) exceeds limit (1024) in function 'vhost_scsi_clear_endpoint' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: drivers/vhost/scsi.c:1831:12: error: stack frame size (1376) exceeds limit (1024) in function 'vhost_scsi_release' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: fs/binfmt_elf.c:766:12: error: stack frame size (1072) exceeds limit (1024) in function 'parse_elf_properties' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: fs/binfmt_elf.c:766:12: error: stack frame size (1088) exceeds limit (1024) in function 'parse_elf_properties' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: fs/select.c:970:12: error: stack frame size (1040) exceeds limit (1024) in function 'do_sys_poll' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: net/core/rtnetlink.c:3251:12: error: stack frame size (1088) exceeds limit (1024) in function '__rtnl_newlink' [-Werror,-Wframe-larger-than]
arm64-archlinux.log: net/sunrpc/auth_gss/gss_krb5_crypto.c:599:1: error: stack frame size (1152) exceeds limit (1024) in function 'gss_krb5_aes_encrypt' [-Werror,-Wframe-larger-than]
arm64-fedora.log: arch/arm64/crypto/aes-neonbs-glue.c:270:12: error: stack frame size (1040) exceeds limit (1024) in function 'aesbs_xts_setkey' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/block/drbd/drbd_main.c:2507:5: error: stack frame size (1088) exceeds limit (1024) in function 'set_resource_options' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/char/ipmi/ipmi_msghandler.c:4850:13: error: stack frame size (1072) exceeds limit (1024) in function 'ipmi_panic_request_and_wait' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/clk/zynqmp/clkc.c:768:12: error: stack frame size (1040) exceeds limit (1024) in function 'zynqmp_clock_probe' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/crypto/ccp/ccp-ops.c:629:1: error: stack frame size (1056) exceeds limit (1024) in function 'ccp_run_aes_gcm_cmd' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/firmware/tegra/bpmp-debugfs.c:294:12: error: stack frame size (1296) exceeds limit (1024) in function 'bpmp_debug_show' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/firmware/tegra/bpmp-debugfs.c:357:16: error: stack frame size (1328) exceeds limit (1024) in function 'bpmp_debug_store' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1779:5: error: stack frame size (1152) exceeds limit (1024) in function 'arm_smmu_atc_inv_domain' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1851:13: error: stack frame size (1136) exceeds limit (1024) in function '__arm_smmu_tlb_inv_range' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:2295:13: error: stack frame size (1136) exceeds limit (1024) in function 'arm_smmu_disable_ats' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:952:13: error: stack frame size (1152) exceeds limit (1024) in function 'arm_smmu_sync_cd' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3308:12: error: stack frame size (8400) exceeds limit (1024) in function 'dpaa_eth_probe' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:534:12: error: stack frame size (4208) exceeds limit (1024) in function 'dpaa_set_coalesce' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/net/wireguard/allowedips.c:255:6: error: stack frame size (1120) exceeds limit (1024) in function 'wg_allowedips_free' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/net/wireguard/allowedips.c:53:13: error: stack frame size (1088) exceeds limit (1024) in function 'root_free_rcu' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/net/wireless/ath/ath11k/mac.c:2539:12: error: stack frame size (1040) exceeds limit (1024) in function 'ath11k_mac_op_hw_scan' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/vhost/scsi.c:1543:1: error: stack frame size (1152) exceeds limit (1024) in function 'vhost_scsi_set_endpoint' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/vhost/scsi.c:1670:1: error: stack frame size (1136) exceeds limit (1024) in function 'vhost_scsi_clear_endpoint' [-Werror,-Wframe-larger-than]
arm64-fedora.log: drivers/vhost/scsi.c:1831:12: error: stack frame size (1360) exceeds limit (1024) in function 'vhost_scsi_release' [-Werror,-Wframe-larger-than]
arm64-fedora.log: fs/binfmt_elf.c:766:12: error: stack frame size (1072) exceeds limit (1024) in function 'parse_elf_properties' [-Werror,-Wframe-larger-than]
arm64-fedora.log: fs/jffs2/xattr.c:775:6: error: stack frame size (1168) exceeds limit (1024) in function 'jffs2_build_xattr_subsystem' [-Werror,-Wframe-larger-than]
arm64-fedora.log: fs/select.c:970:12: error: stack frame size (1040) exceeds limit (1024) in function 'do_sys_poll' [-Werror,-Wframe-larger-than]
arm64-fedora.log: include/linux/module.h:76:12: error: stack frame size (1120) exceeds limit (1024) in function 'init_module' [-Werror,-Wframe-larger-than]
arm64-fedora.log: kernel/cgroup/cpuset.c:1536:12: error: stack frame size (1600) exceeds limit (1024) in function 'update_cpumask' [-Werror,-Wframe-larger-than]
arm64-fedora.log: kernel/cgroup/cpuset.c:1985:12: error: stack frame size (1712) exceeds limit (1024) in function 'update_prstate' [-Werror,-Wframe-larger-than]
arm64-fedora.log: kernel/cgroup/cpuset.c:3200:13: error: stack frame size (1632) exceeds limit (1024) in function 'cpuset_hotplug_workfn' [-Werror,-Wframe-larger-than]
arm64-fedora.log: kernel/irq/affinity.c:338:12: error: stack frame size (1136) exceeds limit (1024) in function 'irq_build_affinity_masks' [-Werror,-Wframe-larger-than]
arm64-fedora.log: kernel/sched/core.c:7934:1: error: stack frame size (1088) exceeds limit (1024) in function '__sched_setaffinity' [-Werror,-Wframe-larger-than]
arm64-fedora.log: kernel/sched/isolation.c:80:19: error: stack frame size (1088) exceeds limit (1024) in function 'housekeeping_setup' [-Werror,-Wframe-larger-than]
arm64-fedora.log: net/core/rtnetlink.c:3251:12: error: stack frame size (1088) exceeds limit (1024) in function '__rtnl_newlink' [-Werror,-Wframe-larger-than]
arm64-fedora.log: net/sunrpc/auth_gss/gss_krb5_crypto.c:599:1: error: stack frame size (1152) exceeds limit (1024) in function 'gss_krb5_aes_encrypt' [-Werror,-Wframe-larger-than]

It appears that both Arch Linux and Fedora define CONFIG_FRAME_WARN
as 1024, below its default of 2048. I am not sure these look particurly
scary (although there are some that are rather large that need to be
looked at).



i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:452:13: error: stack frame size (1628) exceeds limit (1024) in function 'dcn_bw_calc_rq_dlg_ttu' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:1085:13: error: stack frame size (1356) exceeds limit (1024) in function 'dml20_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.c:3286:6: error: stack frame size (1484) exceeds limit (1024) in function 'dml20_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20v2.c:1145:13: error: stack frame size (1228) exceeds limit (1024) in function 'dml20v2_DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20v2.c:3393:6: error: stack frame size (1372) exceeds limit (1024) in function 'dml20v2_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:1466:13: error: stack frame size (1308) exceeds limit (1024) in function 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:3397:6: error: stack frame size (1564) exceeds limit (1024) in function 'dml21_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.c:1657:6: error: stack frame size (1100) exceeds limit (1024) in function 'dml21_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.c:829:13: error: stack frame size (1084) exceeds limit (1024) in function 'dml_rq_dlg_get_dlg_params' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.c:1831:6: error: stack frame size (1108) exceeds limit (1024) in function 'dml30_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.c:981:13: error: stack frame size (1148) exceeds limit (1024) in function 'dml_rq_dlg_get_dlg_params' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_rq_dlg_calc_31.c:939:13: error: stack frame size (1372) exceeds limit (1024) in function 'dml_rq_dlg_get_dlg_params' [-Werror,-Wframe-larger-than]
i386-debian.log: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dml1_display_rq_dlg_calc.c:997:6: error: stack frame size (1212) exceeds limit (1024) in function 'dml1_rq_dlg_get_dlg_params' [-Werror,-Wframe-larger-than]

I am guessing these are all excessive due to the floating point logic
and more inlining. I have investigated some of these previously due to
prior reports:

https://github.com/ClangBuiltLinux/linux/issues/693
https://github.com/ClangBuiltLinux/linux/issues/694
https://github.com/ClangBuiltLinux/linux/issues/695



powerpc64le-debian.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:1918:13: error: stack frame size (2208) exceeds limit (2048) in function 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
powerpc64le-debian.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:3644:6: error: stack frame size (2496) exceeds limit (2048) in function 'dml30_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
powerpc64le-debian.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3956:6: error: stack frame size (2720) exceeds limit (2048) in function 'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
powerpc64le-fedora.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:1918:13: error: stack frame size (2208) exceeds limit (2048) in function 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
powerpc64le-fedora.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:3644:6: error: stack frame size (2496) exceeds limit (2048) in function 'dml30_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
powerpc64le-fedora.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3956:6: error: stack frame size (2720) exceeds limit (2048) in function 'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
powerpc64le-opensuse.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:1918:13: error: stack frame size (2208) exceeds limit (2048) in function 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation' [-Werror,-Wframe-larger-than]
powerpc64le-opensuse.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:3644:6: error: stack frame size (2496) exceeds limit (2048) in function 'dml30_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]
powerpc64le-opensuse.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3956:6: error: stack frame size (2720) exceeds limit (2048) in function 'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than]


I suspect this is a backend problem because these do not really appear
in any other configurations (maybe with Altivec?)



powerpc64le-debian.log:drivers/net/ethernet/sfc/falcon/farch.c:994:10: error: shift count is negative [-Werror,-Wshift-count-negative]
powerpc64le-debian.log:drivers/net/ethernet/sfc/farch.c:985:10: error: shift count is negative [-Werror,-Wshift-count-negative]
powerpc64le-opensuse.log:drivers/net/ethernet/sfc/falcon/farch.c:994:10: error: shift count is negative [-Werror,-Wshift-count-negative]
powerpc64le-opensuse.log:drivers/net/ethernet/sfc/farch.c:985:10: error: shift count is negative [-Werror,-Wshift-count-negative]

I believe this is a false positive due to a bug with how clang models
asm goto in the control flow graph: https://bugs.llvm.org/show_bug.cgi?id=51682



riscv-allmodconfig.log:crypto/ecc.c:1276:13: error: stack frame size (3392) exceeds limit (2048) in function 'ecc_point_mult' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:crypto/ecc.c:1358:6: error: stack frame size (3168) exceeds limit (2048) in function 'ecc_point_mult_shamir' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/block/drbd/drbd_receiver.c:924:12: error: stack frame size (2080) exceeds limit (2048) in function 'conn_connect' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/block/loop.c:1818:12: error: stack frame size (2592) exceeds limit (2048) in function 'lo_ioctl' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/crypto/ccree/cc_hash.c:1882:5: error: stack frame size (2528) exceeds limit (2048) in function 'cc_init_hash_sram' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.c:567:1: error: stack frame size (3520) exceeds limit (2048) in function 'gf100_ram_new_' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c:1521:1: error: stack frame size (5856) exceeds limit (2048) in function 'gk104_ram_new_' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c:940:1: error: stack frame size (2624) exceeds limit (2048) in function 'gt215_ram_new' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/gpu/drm/rockchip/cdn-dp-core.c:1126:12: error: unused function 'cdn_dp_resume' [-Werror,-Wunused-function]
riscv-allmodconfig.log:drivers/hwmon/occ/common.c:1150:5: error: stack frame size (3008) exceeds limit (2048) in function 'occ_setup' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20736) exceeds limit (2048) in function 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/media/common/cx2341x.c:1574:5: error: stack frame size (2944) exceeds limit (2048) in function 'cx2341x_handler_init' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/media/i2c/cx25840/cx25840-core.c:2294:12: error: stack frame size (2976) exceeds limit (2048) in function 'cx25840_reset' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/media/i2c/cx25840/cx25840-core.c:5651:13: error: stack frame size (2400) exceeds limit (2048) in function 'cx23888_std_setup' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/media/pci/cx23885/cx23885-dvb.c:1187:12: error: stack frame size (2688) exceeds limit (2048) in function 'dvb_register' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/media/pci/ddbridge/ddbridge-core.c:2365:6: error: stack frame size (2336) exceeds limit (2048) in function 'ddb_ports_init' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/media/usb/dvb-usb-v2/mxl111sf-i2c.c:799:5: error: stack frame size (2208) exceeds limit (2048) in function 'mxl111sf_i2c_xfer' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/media/usb/gspca/sn9c2028.c:802:12: error: stack frame size (3168) exceeds limit (2048) in function 'sd_start' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/mtd/chips/cfi_cmdset_0001.c:1872:12: error: stack frame size (2432) exceeds limit (2048) in function 'cfi_intelext_writev' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/ethernet/intel/i40e/i40e_ddp.c:264:5: error: stack frame size (2368) exceeds limit (2048) in function 'i40e_ddp_load' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wan/slic_ds26522.c:203:12: error: stack frame size (15328) exceeds limit (2048) in function 'slic_ds26522_probe' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/ath/ath11k/qmi.c:2695:13: error: stack frame size (4384) exceeds limit (2048) in function 'ath11k_qmi_driver_event_work' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/atmel/atmel.c:1050:13: error: stack frame size (2656) exceeds limit (2048) in function 'rx_done_irq' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/atmel/atmel.c:1305:5: error: stack frame size (5152) exceeds limit (2048) in function 'atmel_open' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:15430:13: error: stack frame size (2400) exceeds limit (2048) in function 'wlc_phy_workarounds_nphy_gainctrl' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:17019:13: error: stack frame size (6272) exceeds limit (2048) in function 'wlc_phy_workarounds_nphy' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:25631:1: error: stack frame size (3136) exceeds limit (2048) in function 'wlc_phy_cal_txiqlo_nphy' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:28303:1: error: stack frame size (2464) exceeds limit (2048) in function 'wlc_phy_txpwr_index_nphy' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/intel/ipw2x00/ipw2100.c:5471:12: error: stack frame size (2880) exceeds limit (2048) in function 'ipw2100_configure_security' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c:855:5: error: stack frame size (4672) exceeds limit (2048) in function 'iwl_mvm_ftm_start' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/phy/ti/phy-j721e-wiz.c:1133:12: error: stack frame size (2336) exceeds limit (2048) in function 'wiz_probe' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/rtc/rtc-r9701.c:89:12: error: stack frame size (2400) exceeds limit (2048) in function 'r9701_set_datetime' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/scsi/pm8001/pm80xx_hwi.c:3537:12: error: stack frame size (2368) exceeds limit (2048) in function 'mpi_hw_event' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/scsi/qla2xxx/qla_bsg.c:2787:1: error: stack frame size (3296) exceeds limit (2048) in function 'qla2x00_process_vendor_specific' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/staging/media/hantro/hantro_g2_hevc_dec.c:536:5: error: stack frame size (3616) exceeds limit (2048) in function 'hantro_g2_hevc_dec_run' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/staging/rtl8723bs/core/rtw_security.c:1288:5: error: stack frame size (6976) exceeds limit (2048) in function 'rtw_aes_decrypt' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/staging/rtl8723bs/core/rtw_security.c:865:19: error: stack frame size (5536) exceeds limit (2048) in function 'aes_cipher' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/staging/wlan-ng/cfg80211.c:436:12: error: stack frame size (3904) exceeds limit (2048) in function 'prism2_connect' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/usb/misc/sisusbvga/sisusb.c:1878:12: error: stack frame size (3680) exceeds limit (2048) in function 'sisusb_init_gfxcore' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12: error: stack frame size (14400) exceeds limit (2048) in function 'lb035q02_connect' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:fs/io_uring.c:6578:12: error: stack frame size (2112) exceeds limit (2048) in function 'io_issue_sqe' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:fs/ocfs2/dlm/dlmdomain.c:1852:12: error: stack frame size (2272) exceeds limit (2048) in function 'dlm_join_domain' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:fs/ocfs2/dlm/dlmmaster.c:701:28: error: stack frame size (2208) exceeds limit (2048) in function 'dlm_get_lock_resource' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:fs/ocfs2/dlm/dlmrecovery.c:427:12: error: stack frame size (2976) exceeds limit (2048) in function 'dlm_do_recovery' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:include/linux/module.h:76:12: error: stack frame size (2848) exceeds limit (2048) in function 'init_module' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:lib/bitfield_kunit.c:60:20: error: stack frame size (11328) exceeds limit (10240) in function 'test_bitfields_constants' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:lib/test_kasan.c:946:13: error: stack frame size (3104) exceeds limit (2048) in function 'kasan_bitops_generic' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:lib/test_scanf.c:217:20: error: stack frame size (4640) exceeds limit (2048) in function 'numbers_simple' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:net/mac80211/mesh.c:1516:6: error: stack frame size (2272) exceeds limit (2048) in function 'ieee80211_mesh_rx_queued_mgmt' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:net/qrtr/ns.c:661:13: error: stack frame size (2144) exceeds limit (2048) in function 'qrtr_ns_worker' [-Werror,-Wframe-larger-than]
riscv-allmodconfig.log:sound/usb/mixer_s1810c.c:543:5: error: stack frame size (2208) exceeds limit (2048) in function 'snd_sc1810_init_mixer' [-Werror,-Wframe-larger-than]

I suspect this is a backend problem because these do not really appear
in any other configurations (might also be something with a sanitizer?)



s390x-defconfig.log: include/asm-generic/io.h:464:31: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:477:61: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:490:61: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:501:33: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:511:59: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:521:59: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:609:20: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:617:20: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:625:20: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:634:21: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:643:21: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
s390x-defconfig.log: include/asm-generic/io.h:652:21: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]

This affected all s390x configs I test. fs/btrfs force enables W=1 so we
get these. This is known and had a solution rejected at pull time:

https://github.com/ClangBuiltLinux/linux/issues/1285
https://lore.kernel.org/r/20210510145234.594814-1-schnelle@linux.ibm.com/
https://lore.kernel.org/r/CAK8P3a2oZ-+qd3Nhpy9VVXCJB3DU5N-y-ta2JpP0t6NHh=GVXw@mail.gmail.com/


s390x-allmodconfig.log:drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5184) exceeds limit (2048) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]

Same deal as the other dc_calcs.c warnings.



x86_64-allmodconfig-O3.log:drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c:566:5: error: stack frame size (2504) exceeds limit (2048) in function 'sparx5_config_dsm_calendar' [-Werror,-Wframe-larger-than]

Probably aggressive inlining due to testing -O3.

x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:452:13: error: stack frame size (1800) exceeds limit (1280) in function 'dcn_bw_calc_rq_dlg_ttu' [-Werror,-Wframe-larger-than]
x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.c:1657:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml21_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.c:1831:6: error: stack frame size (1352) exceeds limit (1280) in function 'dml30_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_rq_dlg_calc_31.c:1676:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml31_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
x86_64-alpine.log:drivers/vhost/scsi.c:1831:12: error: stack frame size (1320) exceeds limit (1280) in function 'vhost_scsi_release' [-Werror,-Wframe-larger-than]

Another instance where distros lower CONFIG_FRAME_WARN below the 2048
default. Again, none look particularly scary but should still probably
be dealt with.

Cheers,
Nathan

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Arnd Bergmann]

On Tue, Sep 7, 2021 at 4:32 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> arm32-allmodconfig.log: crypto/wp512.c:782:13: error: stack frame size (1176) exceeds limit (1024) in function 'wp512_process_buffer' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/firmware/tegra/bpmp-debugfs.c:294:12: error: stack frame size (1256) exceeds limit (1024) in function 'bpmp_debug_show' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/firmware/tegra/bpmp-debugfs.c:357:16: error: stack frame size (1264) exceeds limit (1024) in function 'bpmp_debug_store' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1384) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5560) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/mtd/chips/cfi_cmdset_0001.c:1872:12: error: stack frame size (1064) exceeds limit (1024) in function 'cfi_intelext_writev' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/ntb/hw/idt/ntb_hw_idt.c:1041:27: error: stack frame size (1032) exceeds limit (1024) in function 'idt_scan_mws' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/staging/fbtft/fbtft-core.c:902:12: error: stack frame size (1072) exceeds limit (1024) in function 'fbtft_init_display_from_property' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/staging/fbtft/fbtft-core.c:992:5: error: stack frame size (1064) exceeds limit (1024) in function 'fbtft_init_display' [-Werror,-Wframe-larger-than]
> arm32-allmodconfig.log: drivers/staging/rtl8723bs/core/rtw_security.c:1288:5: error: stack frame size (1040) exceeds limit (1024) in function 'rtw_aes_decrypt' [-Werror,-Wframe-larger-than]
> arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1376) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
> arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5384) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]
>
> Aside from the dce_calcs.c warnings, these do not seem too bad. I
> believe allmodconfig turns on UBSAN but it could also be aggressive
> inlining by clang. I intend to look at all -Wframe-large-than warnings
> closely later.

I've had them close to zero in the past, but a couple of new ones came in.

The amdgpu ones are probably not fixable unless they stop using 64-bit
floats in the kernel for
random calculations. The crypto/* ones tend to be compiler bugs, but hard to fix

> It appears that both Arch Linux and Fedora define CONFIG_FRAME_WARN
> as 1024, below its default of 2048. I am not sure these look particurly
> scary (although there are some that are rather large that need to be
> looked at).

For 64-bit, you usually need 1280 bytes stack space to get a
reasonably clean build,
anything that uses more than that tends to be a bug in the code but we
never warned
about those by default as the default warning limit in defconfig is 2048.

I think the distros using 1024 did that because they use a common base config
for 32-bit and 64-bit targets.

> I suspect this is a backend problem because these do not really appear
> in any other configurations (might also be something with a sanitizer?)

Agreed. Someone needs to bisect the .config or the compiler flags to see what
triggers them.

> s390x-defconfig.log: include/asm-generic/io.h:464:31: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:477:61: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:490:61: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:501:33: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:511:59: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:521:59: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:609:20: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:617:20: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:625:20: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:634:21: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:643:21: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> s390x-defconfig.log: include/asm-generic/io.h:652:21: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
>
> This affected all s390x configs I test. fs/btrfs force enables W=1 so we
> get these. This is known and had a solution rejected at pull time:
>
> https://github.com/ClangBuiltLinux/linux/issues/1285
> https://lore.kernel.org/r/20210510145234.594814-1-schnelle@linux.ibm.com/
> https://lore.kernel.org/r/CAK8P3a2oZ-+qd3Nhpy9VVXCJB3DU5N-y-ta2JpP0t6NHh=GVXw@mail.gmail.com/

I posted a new idea for a patch, but it needs more work. I'm happy to work with
any volunteers that want to help tighten the Kconfig dependencies to ensure that
those drivers are only built on architectures that provide I/O port accesses.

> x86_64-allmodconfig-O3.log:drivers/net/ethernet/microchip/sparx5/sparx5_calendar.c:566:5: error: stack frame size (2504) exceeds limit (2048) in function 'sparx5_config_dsm_calendar' [-Werror,-Wframe-larger-than]
>
> Probably aggressive inlining due to testing -O3.

If inlining causes it, it was already bad without the inlining. It
looks like there are
some large arrays on the stack of some of the called functions, so a driver fix
is needed anyway.

> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:452:13: error: stack frame size (1800) exceeds limit (1280) in function 'dcn_bw_calc_rq_dlg_ttu' [-Werror,-Wframe-larger-than]
> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.c:1657:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml21_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.c:1831:6: error: stack frame size (1352) exceeds limit (1280) in function 'dml30_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_rq_dlg_calc_31.c:1676:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml31_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
> x86_64-alpine.log:drivers/vhost/scsi.c:1831:12: error: stack frame size (1320) exceeds limit (1280) in function 'vhost_scsi_release' [-Werror,-Wframe-larger-than]
>
> Another instance where distros lower CONFIG_FRAME_WARN below the 2048
> default. Again, none look particularly scary but should still probably
> be dealt with.

I would argue that they are still scary and should be addressed in the
code, it's just that
we don't see them on build bots that use the 2048 byte default.

        Arnd

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Tue, Sep 7, 2021 at 2:11 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> > x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:452:13: error: stack frame size (1800) exceeds limit (1280) in function 'dcn_bw_calc_rq_dlg_ttu' [-Werror,-Wframe-larger-than]
> > x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.c:1657:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml21_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
> > x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.c:1831:6: error: stack frame size (1352) exceeds limit (1280) in function 'dml30_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
> > x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_rq_dlg_calc_31.c:1676:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml31_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
> > x86_64-alpine.log:drivers/vhost/scsi.c:1831:12: error: stack frame size (1320) exceeds limit (1280) in function 'vhost_scsi_release' [-Werror,-Wframe-larger-than]
> >
> > Another instance where distros lower CONFIG_FRAME_WARN below the 2048
> > default. Again, none look particularly scary but should still probably
> > be dealt with.
>
> I would argue that they are still scary and should be addressed in the
> code, it's just that we don't see them on build bots that use the 2048 byte default.

No, they are scary for another reason entirely: clang is clearly doing
a *HORRIBLE* job with stack usage.

To take that dml30_rq_dlg_get_dlg_reg() function as an example: yes,
it has a few structures on the stack, but gcc allocates 512-720 bytes
of stack space depending on my config.

Not 1280 bytes.

So it's not even *close* to the 1024 byte limit with gcc, much less the 2kB one.

I don't know why clang basically decides to use almost double the
stack space. Maybe it's some other config option that does it, I tried
a fairly normal one and a "almost everythign enabled" one, and
couldn't get close to the reported stack frame size with gcc.

Just to try to make things as close as possible, I tried with the
exact same normal non-debug config (apart from obvious
compiler-dependent things), and picked that dml30_rq_dlg_get_dlg_reg()
function to look at (for no real reason other than that the stack
frame was biggest above.

Gcc did a 720-byte stack frame for that case. Not great, but whatever.

clang did a 1136-byte stack frame for the same thing.

Do I know why? No. I do note that that code is disgusting.

It's passing one of those structs around by value, for example. That's
a 72-byte structure that is copied on the stack due to stupid calling
conventions. Maybe clang generates a few extra temporaries for it as
part of the function call stack setup? Who knows..

                 Linus

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 2203 bytes --]

On Tue, Sep 7, 2021 at 10:10 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Do I know why? No. I do note that that code is disgusting.
>
> It's passing one of those structs around by value, for example. That's
> a 72-byte structure that is copied on the stack due to stupid calling
> conventions. Maybe clang generates a few extra temporaries for it as
> part of the function call stack setup? Who knows..

Ooh, yes.

This attached patch is crap - it converts the helper functions to use
const pointers instead of passing the whole structure, but it then
only converts that one file that *uses* them.

So the end result will not compile in general, but you can do

    make drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.o

and it compiles for me.

And while gcc doesn't care that much - it will apparently either
generate the argument stack every call - clang cares deeply.

The nasty 720-byte stack frame that clang generates turns into just a
320-byte one, and code generation in general looks a *lot* better.

Now, as mentioned, this patch is broken and incomplete. But I really
think the AMD GPU people need to do this. It makes those functions go
from practically unusable to not horribly disgusting.

So Harry/Leo/Alex/Christian and amd-gfx list - can you look into
making this ugly "make one file compile better" patch actually work
properly?

It *looks* lto me ike that code was perhaps written for a C++ compiler
and the helpers have been written as a "pass by reference", and the
arguments used to be

     const display_data_rq_misc_params_st& rq_misc_param

and then the compiler will pass the argument as a pointer. And then it
was converted to C, and the "pass by reference" in the function
declaration was turned into "pass by value", to avoid changing "." to
"->" in the use.

But a '&arg' thing in C++ really is a '*arg' pointer in C, and should
have been done as that.

Of course, it's also possible that that code was simply written by
somebody who didn't understand just *how* horrible it is to pass
structures bigger than a word or two by value.

Do we have a compiler warning for passing big structures by value?

                   Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 17439 bytes --]

 .../display/dc/dml/dcn30/display_rq_dlg_calc_30.c  | 142 ++++++++++-----------
 .../display/dc/dml/dcn30/display_rq_dlg_calc_30.h  |   2 +-
 .../amd/display/dc/dml/display_rq_dlg_helpers.h    |  20 +--
 3 files changed, 82 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.c b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.c
index 0d934fae1c3a..9b5ee53d2de3 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.c
@@ -92,7 +92,7 @@ static void extract_rq_sizing_regs(struct display_mode_lib *mode_lib,
 	const display_data_rq_sizing_params_st rq_sizing)
 {
 	dml_print("DML_DLG: %s: rq_sizing param\n", __func__);
-	print__data_rq_sizing_params_st(mode_lib, rq_sizing);
+	print__data_rq_sizing_params_st(mode_lib, &rq_sizing);
 
 	rq_regs->chunk_size = dml_log2(rq_sizing.chunk_bytes) - 10;
 
@@ -113,28 +113,28 @@ static void extract_rq_sizing_regs(struct display_mode_lib *mode_lib,
 
 static void extract_rq_regs(struct display_mode_lib *mode_lib,
 	display_rq_regs_st *rq_regs,
-	const display_rq_params_st rq_param)
+	const display_rq_params_st *rq_param)
 {
 	unsigned int detile_buf_size_in_bytes = mode_lib->ip.det_buffer_size_kbytes * 1024;
 	unsigned int detile_buf_plane1_addr = 0;
 
-	extract_rq_sizing_regs(mode_lib, &(rq_regs->rq_regs_l), rq_param.sizing.rq_l);
+	extract_rq_sizing_regs(mode_lib, &(rq_regs->rq_regs_l), rq_param->sizing.rq_l);
 
-	rq_regs->rq_regs_l.pte_row_height_linear = dml_floor(dml_log2(rq_param.dlg.rq_l.dpte_row_height),
+	rq_regs->rq_regs_l.pte_row_height_linear = dml_floor(dml_log2(rq_param->dlg.rq_l.dpte_row_height),
 		1) - 3;
 
-	if (rq_param.yuv420) {
-		extract_rq_sizing_regs(mode_lib, &(rq_regs->rq_regs_c), rq_param.sizing.rq_c);
-		rq_regs->rq_regs_c.pte_row_height_linear = dml_floor(dml_log2(rq_param.dlg.rq_c.dpte_row_height),
+	if (rq_param->yuv420) {
+		extract_rq_sizing_regs(mode_lib, &(rq_regs->rq_regs_c), rq_param->sizing.rq_c);
+		rq_regs->rq_regs_c.pte_row_height_linear = dml_floor(dml_log2(rq_param->dlg.rq_c.dpte_row_height),
 			1) - 3;
 	}
 
-	rq_regs->rq_regs_l.swath_height = dml_log2(rq_param.dlg.rq_l.swath_height);
-	rq_regs->rq_regs_c.swath_height = dml_log2(rq_param.dlg.rq_c.swath_height);
+	rq_regs->rq_regs_l.swath_height = dml_log2(rq_param->dlg.rq_l.swath_height);
+	rq_regs->rq_regs_c.swath_height = dml_log2(rq_param->dlg.rq_c.swath_height);
 
 	// FIXME: take the max between luma, chroma chunk size?
 	// okay for now, as we are setting chunk_bytes to 8kb anyways
-	if (rq_param.sizing.rq_l.chunk_bytes >= 32 * 1024 || (rq_param.yuv420 && rq_param.sizing.rq_c.chunk_bytes >= 32 * 1024)) { //32kb
+	if (rq_param->sizing.rq_l.chunk_bytes >= 32 * 1024 || (rq_param->yuv420 && rq_param->sizing.rq_c.chunk_bytes >= 32 * 1024)) { //32kb
 		rq_regs->drq_expansion_mode = 0;
 	} else {
 		rq_regs->drq_expansion_mode = 2;
@@ -143,9 +143,9 @@ static void extract_rq_regs(struct display_mode_lib *mode_lib,
 	rq_regs->mrq_expansion_mode = 1;
 	rq_regs->crq_expansion_mode = 1;
 
-	if (rq_param.yuv420) {
-	if ((double)rq_param.misc.rq_l.stored_swath_bytes
-			/ (double)rq_param.misc.rq_c.stored_swath_bytes <= 1.5) {
+	if (rq_param->yuv420) {
+	if ((double)rq_param->misc.rq_l.stored_swath_bytes
+			/ (double)rq_param->misc.rq_c.stored_swath_bytes <= 1.5) {
 			detile_buf_plane1_addr = (detile_buf_size_in_bytes / 2.0 / 64.0); // half to chroma
 		} else {
 			detile_buf_plane1_addr = dml_round_to_multiple((unsigned int)((2.0 * detile_buf_size_in_bytes) / 3.0),
@@ -747,7 +747,7 @@ static void get_surf_rq_param(struct display_mode_lib *mode_lib,
 	display_data_rq_sizing_params_st *rq_sizing_param,
 	display_data_rq_dlg_params_st *rq_dlg_param,
 	display_data_rq_misc_params_st *rq_misc_param,
-	const display_pipe_params_st pipe_param,
+	const display_pipe_params_st *pipe_param,
 	bool is_chroma,
 	bool is_alpha)
 {
@@ -761,32 +761,32 @@ static void get_surf_rq_param(struct display_mode_lib *mode_lib,
 
 	// FIXME check if ppe apply for both luma and chroma in 422 case
 	if (is_chroma | is_alpha) {
-		vp_width = pipe_param.src.viewport_width_c / ppe;
-		vp_height = pipe_param.src.viewport_height_c;
-		data_pitch = pipe_param.src.data_pitch_c;
-		meta_pitch = pipe_param.src.meta_pitch_c;
-		surface_height = pipe_param.src.surface_height_y / 2.0;
+		vp_width = pipe_param->src.viewport_width_c / ppe;
+		vp_height = pipe_param->src.viewport_height_c;
+		data_pitch = pipe_param->src.data_pitch_c;
+		meta_pitch = pipe_param->src.meta_pitch_c;
+		surface_height = pipe_param->src.surface_height_y / 2.0;
 	} else {
-		vp_width = pipe_param.src.viewport_width / ppe;
-		vp_height = pipe_param.src.viewport_height;
-		data_pitch = pipe_param.src.data_pitch;
-		meta_pitch = pipe_param.src.meta_pitch;
-		surface_height = pipe_param.src.surface_height_y;
+		vp_width = pipe_param->src.viewport_width / ppe;
+		vp_height = pipe_param->src.viewport_height;
+		data_pitch = pipe_param->src.data_pitch;
+		meta_pitch = pipe_param->src.meta_pitch;
+		surface_height = pipe_param->src.surface_height_y;
 	}
 
-	if (pipe_param.dest.odm_combine) {
+	if (pipe_param->dest.odm_combine) {
 		unsigned int access_dir = 0;
 		unsigned int full_src_vp_width = 0;
 		unsigned int hactive_odm = 0;
 		unsigned int src_hactive_odm = 0;
-		access_dir = (pipe_param.src.source_scan == dm_vert); // vp access direction: horizontal or vertical accessed
-		hactive_odm  = pipe_param.dest.hactive / ((unsigned int)pipe_param.dest.odm_combine*2);
+		access_dir = (pipe_param->src.source_scan == dm_vert); // vp access direction: horizontal or vertical accessed
+		hactive_odm  = pipe_param->dest.hactive / ((unsigned int)pipe_param->dest.odm_combine*2);
 		if (is_chroma) {
-			full_src_vp_width = pipe_param.scale_ratio_depth.hscl_ratio_c * pipe_param.dest.full_recout_width;
-			src_hactive_odm  = pipe_param.scale_ratio_depth.hscl_ratio_c * hactive_odm;
+			full_src_vp_width = pipe_param->scale_ratio_depth.hscl_ratio_c * pipe_param->dest.full_recout_width;
+			src_hactive_odm  = pipe_param->scale_ratio_depth.hscl_ratio_c * hactive_odm;
 		} else {
-			full_src_vp_width = pipe_param.scale_ratio_depth.hscl_ratio * pipe_param.dest.full_recout_width;
-			src_hactive_odm  = pipe_param.scale_ratio_depth.hscl_ratio * hactive_odm;
+			full_src_vp_width = pipe_param->scale_ratio_depth.hscl_ratio * pipe_param->dest.full_recout_width;
+			src_hactive_odm  = pipe_param->scale_ratio_depth.hscl_ratio * hactive_odm;
 		}
 
 		if (access_dir == 0) {
@@ -815,7 +815,7 @@ static void get_surf_rq_param(struct display_mode_lib *mode_lib,
 	rq_sizing_param->meta_chunk_bytes = 2048;
 	rq_sizing_param->min_meta_chunk_bytes = 256;
 
-	if (pipe_param.src.hostvm)
+	if (pipe_param->src.hostvm)
 		rq_sizing_param->mpte_group_bytes = 512;
 	else
 		rq_sizing_param->mpte_group_bytes = 2048;
@@ -828,28 +828,28 @@ static void get_surf_rq_param(struct display_mode_lib *mode_lib,
 		vp_height,
 		data_pitch,
 		meta_pitch,
-		pipe_param.src.source_format,
-		pipe_param.src.sw_mode,
-		pipe_param.src.macro_tile_size,
-		pipe_param.src.source_scan,
-		pipe_param.src.hostvm,
+		pipe_param->src.source_format,
+		pipe_param->src.sw_mode,
+		pipe_param->src.macro_tile_size,
+		pipe_param->src.source_scan,
+		pipe_param->src.hostvm,
 		is_chroma,
 		surface_height);
 }
 
 static void dml_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib,
 	display_rq_params_st *rq_param,
-	const display_pipe_params_st pipe_param)
+	const display_pipe_params_st *pipe_param)
 {
 	// get param for luma surface
-	rq_param->yuv420 = pipe_param.src.source_format == dm_420_8
-	|| pipe_param.src.source_format == dm_420_10
-	|| pipe_param.src.source_format == dm_rgbe_alpha
-	|| pipe_param.src.source_format == dm_420_12;
+	rq_param->yuv420 = pipe_param->src.source_format == dm_420_8
+	|| pipe_param->src.source_format == dm_420_10
+	|| pipe_param->src.source_format == dm_rgbe_alpha
+	|| pipe_param->src.source_format == dm_420_12;
 
-	rq_param->yuv420_10bpc = pipe_param.src.source_format == dm_420_10;
+	rq_param->yuv420_10bpc = pipe_param->src.source_format == dm_420_10;
 
-	rq_param->rgbe_alpha = (pipe_param.src.source_format == dm_rgbe_alpha)?1:0;
+	rq_param->rgbe_alpha = (pipe_param->src.source_format == dm_rgbe_alpha)?1:0;
 
 	get_surf_rq_param(mode_lib,
 		&(rq_param->sizing.rq_l),
@@ -859,7 +859,7 @@ static void dml_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib,
 		0,
 		0);
 
-	if (is_dual_plane((enum source_format_class)(pipe_param.src.source_format))) {
+	if (is_dual_plane((enum source_format_class)(pipe_param->src.source_format))) {
 		// get param for chroma surface
 		get_surf_rq_param(mode_lib,
 			&(rq_param->sizing.rq_c),
@@ -871,21 +871,21 @@ static void dml_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib,
 	}
 
 	// calculate how to split the det buffer space between luma and chroma
-	handle_det_buf_split(mode_lib, rq_param, pipe_param.src);
-	print__rq_params_st(mode_lib, *rq_param);
+	handle_det_buf_split(mode_lib, rq_param, pipe_param->src);
+	print__rq_params_st(mode_lib, rq_param);
 }
 
 void dml30_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 	display_rq_regs_st *rq_regs,
-	const display_pipe_params_st pipe_param)
+	const display_pipe_params_st *pipe_param)
 {
 	display_rq_params_st rq_param = { 0 };
 
 	memset(rq_regs, 0, sizeof(*rq_regs));
 	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, pipe_param);
-	extract_rq_regs(mode_lib, rq_regs, rq_param);
+	extract_rq_regs(mode_lib, rq_regs, &rq_param);
 
-	print__rq_regs_st(mode_lib, *rq_regs);
+	print__rq_regs_st(mode_lib, rq_regs);
 }
 
 static void calculate_ttu_cursor(struct display_mode_lib *mode_lib,
@@ -984,8 +984,8 @@ static void dml_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib,
 	const unsigned int pipe_idx,
 	display_dlg_regs_st *disp_dlg_regs,
 	display_ttu_regs_st *disp_ttu_regs,
-	const display_rq_dlg_params_st rq_dlg_param,
-	const display_dlg_sys_params_st dlg_sys_param,
+	const display_rq_dlg_params_st *rq_dlg_param,
+	const display_dlg_sys_params_st *dlg_sys_param,
 	const bool cstate_en,
 	const bool pstate_en,
 	const bool vm_en,
@@ -1137,7 +1137,7 @@ static void dml_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib,
 		* dml_pow(2, 8));
 	disp_dlg_regs->dlg_vblank_end = interlaced ? (vblank_end / 2) : vblank_end; // 15 bits
 
-	min_dcfclk_mhz = dlg_sys_param.deepsleep_dcfclk_mhz;
+	min_dcfclk_mhz = dlg_sys_param->deepsleep_dcfclk_mhz;
 	t_calc_us = get_tcalc(mode_lib, e2e_pipe_param, num_pipes);
 	min_ttu_vblank = get_min_ttu_vblank(mode_lib, e2e_pipe_param, num_pipes, pipe_idx);
 
@@ -1191,13 +1191,13 @@ static void dml_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib,
 	scl_enable = scl->scl_enable;
 
 	line_time_in_us = (htotal / pclk_freq_in_mhz);
-	swath_width_ub_l = rq_dlg_param.rq_l.swath_width_ub;
-	dpte_groups_per_row_ub_l = rq_dlg_param.rq_l.dpte_groups_per_row_ub;
-	swath_width_ub_c = rq_dlg_param.rq_c.swath_width_ub;
-	dpte_groups_per_row_ub_c = rq_dlg_param.rq_c.dpte_groups_per_row_ub;
+	swath_width_ub_l = rq_dlg_param->rq_l.swath_width_ub;
+	dpte_groups_per_row_ub_l = rq_dlg_param->rq_l.dpte_groups_per_row_ub;
+	swath_width_ub_c = rq_dlg_param->rq_c.swath_width_ub;
+	dpte_groups_per_row_ub_c = rq_dlg_param->rq_c.dpte_groups_per_row_ub;
 
-	meta_chunks_per_row_ub_l = rq_dlg_param.rq_l.meta_chunks_per_row_ub;
-	meta_chunks_per_row_ub_c = rq_dlg_param.rq_c.meta_chunks_per_row_ub;
+	meta_chunks_per_row_ub_l = rq_dlg_param->rq_l.meta_chunks_per_row_ub;
+	meta_chunks_per_row_ub_c = rq_dlg_param->rq_c.meta_chunks_per_row_ub;
 	vupdate_offset = dst->vupdate_offset;
 	vupdate_width = dst->vupdate_width;
 	vready_offset = dst->vready_offset;
@@ -1379,16 +1379,16 @@ static void dml_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib,
 	dml_print("DML_DLG: %s: vratio_pre_c=%3.2f\n", __func__, vratio_pre_c);
 
 	// Active
-	req_per_swath_ub_l = rq_dlg_param.rq_l.req_per_swath_ub;
-	req_per_swath_ub_c = rq_dlg_param.rq_c.req_per_swath_ub;
-	meta_row_height_l = rq_dlg_param.rq_l.meta_row_height;
-	meta_row_height_c = rq_dlg_param.rq_c.meta_row_height;
+	req_per_swath_ub_l = rq_dlg_param->rq_l.req_per_swath_ub;
+	req_per_swath_ub_c = rq_dlg_param->rq_c.req_per_swath_ub;
+	meta_row_height_l = rq_dlg_param->rq_l.meta_row_height;
+	meta_row_height_c = rq_dlg_param->rq_c.meta_row_height;
 	swath_width_pixels_ub_l = 0;
 	swath_width_pixels_ub_c = 0;
 	scaler_rec_in_width_l = 0;
 	scaler_rec_in_width_c = 0;
-	dpte_row_height_l = rq_dlg_param.rq_l.dpte_row_height;
-	dpte_row_height_c = rq_dlg_param.rq_c.dpte_row_height;
+	dpte_row_height_l = rq_dlg_param->rq_l.dpte_row_height;
+	dpte_row_height_c = rq_dlg_param->rq_c.dpte_row_height;
 
 	if (mode_422) {
 		swath_width_pixels_ub_l = swath_width_ub_l * 2;  // *2 for 2 pixel per element
@@ -1824,8 +1824,8 @@ static void dml_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib,
 	disp_ttu_regs->min_ttu_vblank = min_ttu_vblank * refclk_freq_in_mhz;
 	ASSERT(disp_ttu_regs->min_ttu_vblank < dml_pow(2, 24));
 
-	print__ttu_regs_st(mode_lib, *disp_ttu_regs);
-	print__dlg_regs_st(mode_lib, *disp_dlg_regs);
+	print__ttu_regs_st(mode_lib, disp_ttu_regs);
+	print__dlg_regs_st(mode_lib, disp_dlg_regs);
 }
 
 void dml30_rq_dlg_get_dlg_reg(struct display_mode_lib *mode_lib,
@@ -1861,20 +1861,20 @@ void dml30_rq_dlg_get_dlg_reg(struct display_mode_lib *mode_lib,
 	dlg_sys_param.t_srx_delay_us = mode_lib->ip.dcfclk_cstate_latency
 		/ dlg_sys_param.deepsleep_dcfclk_mhz; // TODO: Deprecated
 
-	print__dlg_sys_params_st(mode_lib, dlg_sys_param);
+	print__dlg_sys_params_st(mode_lib, &dlg_sys_param);
 
 	// system parameter calculation done
 
 	dml_print("DML_DLG: Calculation for pipe[%d] start\n\n", pipe_idx);
-	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, e2e_pipe_param[pipe_idx].pipe);
+	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, &e2e_pipe_param[pipe_idx].pipe);
 	dml_rq_dlg_get_dlg_params(mode_lib,
 		e2e_pipe_param,
 		num_pipes,
 		pipe_idx,
 		dlg_regs,
 		ttu_regs,
-		rq_param.dlg,
-		dlg_sys_param,
+		&rq_param.dlg,
+		&dlg_sys_param,
 		cstate_en,
 		pstate_en,
 		vm_en,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.h b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.h
index c04965cceff3..b40abc41828a 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.h
@@ -41,7 +41,7 @@ struct display_mode_lib;
 //            See also: <display_rq_regs_st>
 void dml30_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param);
+		const display_pipe_params_st *pipe_param);
 
 // Function: dml_rq_dlg_get_dlg_reg
 //   Calculate and return DLG and TTU register struct given the system setting
diff --git a/drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_helpers.h b/drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_helpers.h
index 2555ef0358c2..fb61a0b1470f 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_helpers.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_helpers.h
@@ -31,16 +31,16 @@
 /* Function: Printer functions
  *  Print various struct
  */
-void print__rq_params_st(struct display_mode_lib *mode_lib, display_rq_params_st rq_param);
-void print__data_rq_sizing_params_st(struct display_mode_lib *mode_lib, display_data_rq_sizing_params_st rq_sizing);
-void print__data_rq_dlg_params_st(struct display_mode_lib *mode_lib, display_data_rq_dlg_params_st rq_dlg_param);
-void print__data_rq_misc_params_st(struct display_mode_lib *mode_lib, display_data_rq_misc_params_st rq_misc_param);
-void print__rq_dlg_params_st(struct display_mode_lib *mode_lib, display_rq_dlg_params_st rq_dlg_param);
-void print__dlg_sys_params_st(struct display_mode_lib *mode_lib, display_dlg_sys_params_st dlg_sys_param);
+void print__rq_params_st(struct display_mode_lib *mode_lib, const display_rq_params_st *rq_param);
+void print__data_rq_sizing_params_st(struct display_mode_lib *mode_lib, const display_data_rq_sizing_params_st *rq_sizing);
+void print__data_rq_dlg_params_st(struct display_mode_lib *mode_lib, const display_data_rq_dlg_params_st *rq_dlg_param);
+void print__data_rq_misc_params_st(struct display_mode_lib *mode_lib, const display_data_rq_misc_params_st *rq_misc_param);
+void print__rq_dlg_params_st(struct display_mode_lib *mode_lib, const display_rq_dlg_params_st *rq_dlg_param);
+void print__dlg_sys_params_st(struct display_mode_lib *mode_lib, const display_dlg_sys_params_st *dlg_sys_param);
 
-void print__data_rq_regs_st(struct display_mode_lib *mode_lib, display_data_rq_regs_st data_rq_regs);
-void print__rq_regs_st(struct display_mode_lib *mode_lib, display_rq_regs_st rq_regs);
-void print__dlg_regs_st(struct display_mode_lib *mode_lib, display_dlg_regs_st dlg_regs);
-void print__ttu_regs_st(struct display_mode_lib *mode_lib, display_ttu_regs_st ttu_regs);
+void print__data_rq_regs_st(struct display_mode_lib *mode_lib, const display_data_rq_regs_st *data_rq_regs);
+void print__rq_regs_st(struct display_mode_lib *mode_lib, const display_rq_regs_st *rq_regs);
+void print__dlg_regs_st(struct display_mode_lib *mode_lib, const display_dlg_regs_st *dlg_regs);
+void print__ttu_regs_st(struct display_mode_lib *mode_lib, const display_ttu_regs_st *ttu_regs);
 
 #endif

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Harry Wentland]

On 2021-09-07 1:33 p.m., Linus Torvalds wrote:
> On Tue, Sep 7, 2021 at 10:10 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Do I know why? No. I do note that that code is disgusting.
>>
>> It's passing one of those structs around by value, for example. That's
>> a 72-byte structure that is copied on the stack due to stupid calling
>> conventions. Maybe clang generates a few extra temporaries for it as
>> part of the function call stack setup? Who knows..
> 
> Ooh, yes.
> 
> This attached patch is crap - it converts the helper functions to use
> const pointers instead of passing the whole structure, but it then
> only converts that one file that *uses* them.
> 
> So the end result will not compile in general, but you can do
> 
>     make drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.o
> 
> and it compiles for me.
> 
> And while gcc doesn't care that much - it will apparently either
> generate the argument stack every call - clang cares deeply.
> 
> The nasty 720-byte stack frame that clang generates turns into just a
> 320-byte one, and code generation in general looks a *lot* better.
> 
> Now, as mentioned, this patch is broken and incomplete. But I really
> think the AMD GPU people need to do this. It makes those functions go
> from practically unusable to not horribly disgusting.
> 
> So Harry/Leo/Alex/Christian and amd-gfx list - can you look into
> making this ugly "make one file compile better" patch actually work
> properly?
> 

Yes, will take a look at this tonight. We definitely shouldn't be passing
large structs by value.

Harry

> It *looks* lto me ike that code was perhaps written for a C++ compiler
> and the helpers have been written as a "pass by reference", and the
> arguments used to be
> 
>      const display_data_rq_misc_params_st& rq_misc_param
> 
> and then the compiler will pass the argument as a pointer. And then it
> was converted to C, and the "pass by reference" in the function
> declaration was turned into "pass by value", to avoid changing "." to
> "->" in the use.
> 
> But a '&arg' thing in C++ really is a '*arg' pointer in C, and should
> have been done as that.
> 
> Of course, it's also possible that that code was simply written by
> somebody who didn't understand just *how* horrible it is to pass
> structures bigger than a word or two by value.
> 
> Do we have a compiler warning for passing big structures by value?
> 
>                    Linus
> 

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Harry Wentland]

[-- Attachment #1: Type: text/plain, Size: 3680 bytes --]



On 2021-09-07 5:07 p.m., Harry Wentland wrote:
> 
> 
> On 2021-09-07 1:33 p.m., Linus Torvalds wrote:
>> On Tue, Sep 7, 2021 at 10:10 AM Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>>
>>> Do I know why? No. I do note that that code is disgusting.
>>>
>>> It's passing one of those structs around by value, for example. That's
>>> a 72-byte structure that is copied on the stack due to stupid calling
>>> conventions. Maybe clang generates a few extra temporaries for it as
>>> part of the function call stack setup? Who knows..
>>
>> Ooh, yes.
>>
>> This attached patch is crap - it converts the helper functions to use
>> const pointers instead of passing the whole structure, but it then
>> only converts that one file that *uses* them.
>>
>> So the end result will not compile in general, but you can do
>>
>>     make drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.o
>>
>> and it compiles for me.
>>
>> And while gcc doesn't care that much - it will apparently either
>> generate the argument stack every call - clang cares deeply.
>>
>> The nasty 720-byte stack frame that clang generates turns into just a
>> 320-byte one, and code generation in general looks a *lot* better.
>>
>> Now, as mentioned, this patch is broken and incomplete. But I really
>> think the AMD GPU people need to do this. It makes those functions go
>> from practically unusable to not horribly disgusting.
>>
>> So Harry/Leo/Alex/Christian and amd-gfx list - can you look into
>> making this ugly "make one file compile better" patch actually work
>> properly?
>>
> 
> Yes, will take a look at this tonight. We definitely shouldn't be passing
> large structs by value.
> 

Attached patches fix these x86_64 ones reported by Nick:

x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:452:13: error: stack frame size (1800) exceeds limit (1280) in function 'dcn_bw_calc_rq_dlg_ttu' [-Werror,-Wframe-larger-than]
x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.c:1657:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml21_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.c:1831:6: error: stack frame size (1352) exceeds limit (1280) in function 'dml30_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]

I'm also seeing one more that might be more challenging to fix but is nearly at 1024:

drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:3397:6: error: stack frame size of 1064 bytes in function 'dml21_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than=]

The attached patches build and boot without error or warning on a Radeon RX 5500 XT.

Harry

> Harry
> 
>> It *looks* lto me ike that code was perhaps written for a C++ compiler
>> and the helpers have been written as a "pass by reference", and the
>> arguments used to be
>>
>>      const display_data_rq_misc_params_st& rq_misc_param
>>
>> and then the compiler will pass the argument as a pointer. And then it
>> was converted to C, and the "pass by reference" in the function
>> declaration was turned into "pass by value", to avoid changing "." to
>> "->" in the use.
>>
>> But a '&arg' thing in C++ really is a '*arg' pointer in C, and should
>> have been done as that.
>>
>> Of course, it's also possible that that code was simply written by
>> somebody who didn't understand just *how* horrible it is to pass
>> structures bigger than a word or two by value.
>>
>> Do we have a compiler warning for passing big structures by value?
>>
>>                    Linus
>>
> 


[-- Attachment #2: 0001-drm-amd-display-Pass-display_pipe_params_st-as-const.patch --]
[-- Type: text/x-patch, Size: 30495 bytes --]

From 353a23b14584705cb194d9bfc91010caacaf42f8 Mon Sep 17 00:00:00 2001
From: Harry Wentland <harry.wentland@amd.com>
Date: Tue, 7 Sep 2021 19:40:06 -0400
Subject: [PATCH 1/2] drm/amd/display: Pass display_pipe_params_st as const in
 DML

[Why]
This neither needs to be on the stack nor passed by value
to each function call. In fact, when building with clang
it seems to break the Linux's default 1024 byte stack
frame limit.

[How]
We can simply pass this as a const pointer.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 .../drm/amd/display/dc/dcn20/dcn20_resource.c |  2 +-
 .../dc/dml/dcn20/display_rq_dlg_calc_20.c     |  6 +-
 .../dc/dml/dcn20/display_rq_dlg_calc_20.h     |  4 +-
 .../dc/dml/dcn20/display_rq_dlg_calc_20v2.c   |  6 +-
 .../dc/dml/dcn20/display_rq_dlg_calc_20v2.h   |  4 +-
 .../dc/dml/dcn21/display_rq_dlg_calc_21.c     | 62 ++++++++--------
 .../dc/dml/dcn21/display_rq_dlg_calc_21.h     |  4 +-
 .../dc/dml/dcn30/display_rq_dlg_calc_30.c     | 72 +++++++++----------
 .../dc/dml/dcn30/display_rq_dlg_calc_30.h     |  4 +-
 .../dc/dml/dcn31/display_rq_dlg_calc_31.c     | 68 +++++++++---------
 .../dc/dml/dcn31/display_rq_dlg_calc_31.h     |  4 +-
 .../drm/amd/display/dc/dml/display_mode_lib.h |  4 +-
 12 files changed, 120 insertions(+), 120 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index e3e01b17c164..4389b36f0760 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -3152,7 +3152,7 @@ void dcn20_calculate_dlg_params(
 
 		context->bw_ctx.dml.funcs.rq_dlg_get_rq_reg(&context->bw_ctx.dml,
 				&context->res_ctx.pipe_ctx[i].rq_regs,
-				pipes[pipe_idx].pipe);
+				&pipes[pipe_idx].pipe);
 		pipe_idx++;
 	}
 }
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c
index 2091dd8c252d..8c168f348a27 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c
@@ -768,12 +768,12 @@ static void dml20_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib,
 
 void dml20_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param)
+		const display_pipe_params_st *pipe_param)
 {
 	display_rq_params_st rq_param = {0};
 
 	memset(rq_regs, 0, sizeof(*rq_regs));
-	dml20_rq_dlg_get_rq_params(mode_lib, &rq_param, pipe_param.src);
+	dml20_rq_dlg_get_rq_params(mode_lib, &rq_param, pipe_param->src);
 	extract_rq_regs(mode_lib, rq_regs, rq_param);
 
 	print__rq_regs_st(mode_lib, *rq_regs);
@@ -1549,7 +1549,7 @@ static void dml20_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib,
 void dml20_rq_dlg_get_dlg_reg(struct display_mode_lib *mode_lib,
 		display_dlg_regs_st *dlg_regs,
 		display_ttu_regs_st *ttu_regs,
-		display_e2e_pipe_params_st *e2e_pipe_param,
+		const display_e2e_pipe_params_st *e2e_pipe_param,
 		const unsigned int num_pipes,
 		const unsigned int pipe_idx,
 		const bool cstate_en,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.h b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.h
index d0b90947f540..8b23867e97c1 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.h
@@ -43,7 +43,7 @@ struct display_mode_lib;
 void dml20_rq_dlg_get_rq_reg(
 		struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param);
+		const display_pipe_params_st *pipe_param);
 
 
 // Function: dml_rq_dlg_get_dlg_reg
@@ -61,7 +61,7 @@ void dml20_rq_dlg_get_dlg_reg(
 		struct display_mode_lib *mode_lib,
 		display_dlg_regs_st *dlg_regs,
 		display_ttu_regs_st *ttu_regs,
-		display_e2e_pipe_params_st *e2e_pipe_param,
+		const display_e2e_pipe_params_st *e2e_pipe_param,
 		const unsigned int num_pipes,
 		const unsigned int pipe_idx,
 		const bool cstate_en,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20v2.c b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20v2.c
index 1a0c14e465fa..26ececfd40cd 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20v2.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20v2.c
@@ -768,12 +768,12 @@ static void dml20v2_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib,
 
 void dml20v2_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param)
+		const display_pipe_params_st *pipe_param)
 {
 	display_rq_params_st rq_param = {0};
 
 	memset(rq_regs, 0, sizeof(*rq_regs));
-	dml20v2_rq_dlg_get_rq_params(mode_lib, &rq_param, pipe_param.src);
+	dml20v2_rq_dlg_get_rq_params(mode_lib, &rq_param, pipe_param->src);
 	extract_rq_regs(mode_lib, rq_regs, rq_param);
 
 	print__rq_regs_st(mode_lib, *rq_regs);
@@ -1550,7 +1550,7 @@ static void dml20v2_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib,
 void dml20v2_rq_dlg_get_dlg_reg(struct display_mode_lib *mode_lib,
 		display_dlg_regs_st *dlg_regs,
 		display_ttu_regs_st *ttu_regs,
-		display_e2e_pipe_params_st *e2e_pipe_param,
+		const display_e2e_pipe_params_st *e2e_pipe_param,
 		const unsigned int num_pipes,
 		const unsigned int pipe_idx,
 		const bool cstate_en,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20v2.h b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20v2.h
index 27cf8bed9376..2b4e46ea1c3d 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20v2.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20v2.h
@@ -43,7 +43,7 @@ struct display_mode_lib;
 void dml20v2_rq_dlg_get_rq_reg(
 		struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param);
+		const display_pipe_params_st *pipe_param);
 
 
 // Function: dml_rq_dlg_get_dlg_reg
@@ -61,7 +61,7 @@ void dml20v2_rq_dlg_get_dlg_reg(
 		struct display_mode_lib *mode_lib,
 		display_dlg_regs_st *dlg_regs,
 		display_ttu_regs_st *ttu_regs,
-		display_e2e_pipe_params_st *e2e_pipe_param,
+		const display_e2e_pipe_params_st *e2e_pipe_param,
 		const unsigned int num_pipes,
 		const unsigned int pipe_idx,
 		const bool cstate_en,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_rq_dlg_calc_21.c b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_rq_dlg_calc_21.c
index 287e31052b30..736978c4d40a 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_rq_dlg_calc_21.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_rq_dlg_calc_21.c
@@ -694,7 +694,7 @@ static void get_surf_rq_param(
 		display_data_rq_sizing_params_st *rq_sizing_param,
 		display_data_rq_dlg_params_st *rq_dlg_param,
 		display_data_rq_misc_params_st *rq_misc_param,
-		const display_pipe_params_st pipe_param,
+		const display_pipe_params_st *pipe_param,
 		bool is_chroma)
 {
 	bool mode_422 = false;
@@ -706,30 +706,30 @@ static void get_surf_rq_param(
 
 	// FIXME check if ppe apply for both luma and chroma in 422 case
 	if (is_chroma) {
-		vp_width = pipe_param.src.viewport_width_c / ppe;
-		vp_height = pipe_param.src.viewport_height_c;
-		data_pitch = pipe_param.src.data_pitch_c;
-		meta_pitch = pipe_param.src.meta_pitch_c;
+		vp_width = pipe_param->src.viewport_width_c / ppe;
+		vp_height = pipe_param->src.viewport_height_c;
+		data_pitch = pipe_param->src.data_pitch_c;
+		meta_pitch = pipe_param->src.meta_pitch_c;
 	} else {
-		vp_width = pipe_param.src.viewport_width / ppe;
-		vp_height = pipe_param.src.viewport_height;
-		data_pitch = pipe_param.src.data_pitch;
-		meta_pitch = pipe_param.src.meta_pitch;
+		vp_width = pipe_param->src.viewport_width / ppe;
+		vp_height = pipe_param->src.viewport_height;
+		data_pitch = pipe_param->src.data_pitch;
+		meta_pitch = pipe_param->src.meta_pitch;
 	}
 
-	if (pipe_param.dest.odm_combine) {
+	if (pipe_param->dest.odm_combine) {
 		unsigned int access_dir;
 		unsigned int full_src_vp_width;
 		unsigned int hactive_half;
 		unsigned int src_hactive_half;
-		access_dir = (pipe_param.src.source_scan == dm_vert); // vp access direction: horizontal or vertical accessed
-		hactive_half  = pipe_param.dest.hactive / 2;
+		access_dir = (pipe_param->src.source_scan == dm_vert); // vp access direction: horizontal or vertical accessed
+		hactive_half  = pipe_param->dest.hactive / 2;
 		if (is_chroma) {
-			full_src_vp_width = pipe_param.scale_ratio_depth.hscl_ratio_c * pipe_param.dest.full_recout_width;
-			src_hactive_half  = pipe_param.scale_ratio_depth.hscl_ratio_c * hactive_half;
+			full_src_vp_width = pipe_param->scale_ratio_depth.hscl_ratio_c * pipe_param->dest.full_recout_width;
+			src_hactive_half  = pipe_param->scale_ratio_depth.hscl_ratio_c * hactive_half;
 		} else {
-			full_src_vp_width = pipe_param.scale_ratio_depth.hscl_ratio * pipe_param.dest.full_recout_width;
-			src_hactive_half  = pipe_param.scale_ratio_depth.hscl_ratio * hactive_half;
+			full_src_vp_width = pipe_param->scale_ratio_depth.hscl_ratio * pipe_param->dest.full_recout_width;
+			src_hactive_half  = pipe_param->scale_ratio_depth.hscl_ratio * hactive_half;
 		}
 
 		if (access_dir == 0) {
@@ -754,7 +754,7 @@ static void get_surf_rq_param(
 	rq_sizing_param->meta_chunk_bytes = 2048;
 	rq_sizing_param->min_meta_chunk_bytes = 256;
 
-	if (pipe_param.src.hostvm)
+	if (pipe_param->src.hostvm)
 		rq_sizing_param->mpte_group_bytes = 512;
 	else
 		rq_sizing_param->mpte_group_bytes = 2048;
@@ -768,23 +768,23 @@ static void get_surf_rq_param(
 			vp_height,
 			data_pitch,
 			meta_pitch,
-			pipe_param.src.source_format,
-			pipe_param.src.sw_mode,
-			pipe_param.src.macro_tile_size,
-			pipe_param.src.source_scan,
-			pipe_param.src.hostvm,
+			pipe_param->src.source_format,
+			pipe_param->src.sw_mode,
+			pipe_param->src.macro_tile_size,
+			pipe_param->src.source_scan,
+			pipe_param->src.hostvm,
 			is_chroma);
 }
 
 static void dml_rq_dlg_get_rq_params(
 		struct display_mode_lib *mode_lib,
 		display_rq_params_st *rq_param,
-		const display_pipe_params_st pipe_param)
+		const display_pipe_params_st *pipe_param)
 {
 	// get param for luma surface
-	rq_param->yuv420 = pipe_param.src.source_format == dm_420_8
-			|| pipe_param.src.source_format == dm_420_10;
-	rq_param->yuv420_10bpc = pipe_param.src.source_format == dm_420_10;
+	rq_param->yuv420 = pipe_param->src.source_format == dm_420_8
+			|| pipe_param->src.source_format == dm_420_10;
+	rq_param->yuv420_10bpc = pipe_param->src.source_format == dm_420_10;
 
 	get_surf_rq_param(
 			mode_lib,
@@ -794,7 +794,7 @@ static void dml_rq_dlg_get_rq_params(
 			pipe_param,
 			0);
 
-	if (is_dual_plane((enum source_format_class) (pipe_param.src.source_format))) {
+	if (is_dual_plane((enum source_format_class) (pipe_param->src.source_format))) {
 		// get param for chroma surface
 		get_surf_rq_param(
 				mode_lib,
@@ -806,14 +806,14 @@ static void dml_rq_dlg_get_rq_params(
 	}
 
 	// calculate how to split the det buffer space between luma and chroma
-	handle_det_buf_split(mode_lib, rq_param, pipe_param.src);
+	handle_det_buf_split(mode_lib, rq_param, pipe_param->src);
 	print__rq_params_st(mode_lib, *rq_param);
 }
 
 void dml21_rq_dlg_get_rq_reg(
 		struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param)
+		const display_pipe_params_st *pipe_param)
 {
 	display_rq_params_st rq_param = {0};
 
@@ -1658,7 +1658,7 @@ void dml21_rq_dlg_get_dlg_reg(
 		struct display_mode_lib *mode_lib,
 		display_dlg_regs_st *dlg_regs,
 		display_ttu_regs_st *ttu_regs,
-		display_e2e_pipe_params_st *e2e_pipe_param,
+		const display_e2e_pipe_params_st *e2e_pipe_param,
 		const unsigned int num_pipes,
 		const unsigned int pipe_idx,
 		const bool cstate_en,
@@ -1696,7 +1696,7 @@ void dml21_rq_dlg_get_dlg_reg(
 	// system parameter calculation done
 
 	dml_print("DML_DLG: Calculation for pipe[%d] start\n\n", pipe_idx);
-	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, e2e_pipe_param[pipe_idx].pipe);
+	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, &e2e_pipe_param[pipe_idx].pipe);
 	dml_rq_dlg_get_dlg_params(
 			mode_lib,
 			e2e_pipe_param,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_rq_dlg_calc_21.h b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_rq_dlg_calc_21.h
index e8f7785e3fc6..af6ad0ca9cf8 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_rq_dlg_calc_21.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn21/display_rq_dlg_calc_21.h
@@ -44,7 +44,7 @@ struct display_mode_lib;
 void dml21_rq_dlg_get_rq_reg(
 		struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param);
+		const display_pipe_params_st *pipe_param);
 
 // Function: dml_rq_dlg_get_dlg_reg
 //   Calculate and return DLG and TTU register struct given the system setting
@@ -61,7 +61,7 @@ void dml21_rq_dlg_get_dlg_reg(
 		struct display_mode_lib *mode_lib,
 		display_dlg_regs_st *dlg_regs,
 		display_ttu_regs_st *ttu_regs,
-		display_e2e_pipe_params_st *e2e_pipe_param,
+		const display_e2e_pipe_params_st *e2e_pipe_param,
 		const unsigned int num_pipes,
 		const unsigned int pipe_idx,
 		const bool cstate_en,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.c b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.c
index 0d934fae1c3a..2120e0941a09 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.c
@@ -747,7 +747,7 @@ static void get_surf_rq_param(struct display_mode_lib *mode_lib,
 	display_data_rq_sizing_params_st *rq_sizing_param,
 	display_data_rq_dlg_params_st *rq_dlg_param,
 	display_data_rq_misc_params_st *rq_misc_param,
-	const display_pipe_params_st pipe_param,
+	const display_pipe_params_st *pipe_param,
 	bool is_chroma,
 	bool is_alpha)
 {
@@ -761,32 +761,32 @@ static void get_surf_rq_param(struct display_mode_lib *mode_lib,
 
 	// FIXME check if ppe apply for both luma and chroma in 422 case
 	if (is_chroma | is_alpha) {
-		vp_width = pipe_param.src.viewport_width_c / ppe;
-		vp_height = pipe_param.src.viewport_height_c;
-		data_pitch = pipe_param.src.data_pitch_c;
-		meta_pitch = pipe_param.src.meta_pitch_c;
-		surface_height = pipe_param.src.surface_height_y / 2.0;
+		vp_width = pipe_param->src.viewport_width_c / ppe;
+		vp_height = pipe_param->src.viewport_height_c;
+		data_pitch = pipe_param->src.data_pitch_c;
+		meta_pitch = pipe_param->src.meta_pitch_c;
+		surface_height = pipe_param->src.surface_height_y / 2.0;
 	} else {
-		vp_width = pipe_param.src.viewport_width / ppe;
-		vp_height = pipe_param.src.viewport_height;
-		data_pitch = pipe_param.src.data_pitch;
-		meta_pitch = pipe_param.src.meta_pitch;
-		surface_height = pipe_param.src.surface_height_y;
+		vp_width = pipe_param->src.viewport_width / ppe;
+		vp_height = pipe_param->src.viewport_height;
+		data_pitch = pipe_param->src.data_pitch;
+		meta_pitch = pipe_param->src.meta_pitch;
+		surface_height = pipe_param->src.surface_height_y;
 	}
 
-	if (pipe_param.dest.odm_combine) {
+	if (pipe_param->dest.odm_combine) {
 		unsigned int access_dir = 0;
 		unsigned int full_src_vp_width = 0;
 		unsigned int hactive_odm = 0;
 		unsigned int src_hactive_odm = 0;
-		access_dir = (pipe_param.src.source_scan == dm_vert); // vp access direction: horizontal or vertical accessed
-		hactive_odm  = pipe_param.dest.hactive / ((unsigned int)pipe_param.dest.odm_combine*2);
+		access_dir = (pipe_param->src.source_scan == dm_vert); // vp access direction: horizontal or vertical accessed
+		hactive_odm  = pipe_param->dest.hactive / ((unsigned int) pipe_param->dest.odm_combine*2);
 		if (is_chroma) {
-			full_src_vp_width = pipe_param.scale_ratio_depth.hscl_ratio_c * pipe_param.dest.full_recout_width;
-			src_hactive_odm  = pipe_param.scale_ratio_depth.hscl_ratio_c * hactive_odm;
+			full_src_vp_width = pipe_param->scale_ratio_depth.hscl_ratio_c * pipe_param->dest.full_recout_width;
+			src_hactive_odm  = pipe_param->scale_ratio_depth.hscl_ratio_c * hactive_odm;
 		} else {
-			full_src_vp_width = pipe_param.scale_ratio_depth.hscl_ratio * pipe_param.dest.full_recout_width;
-			src_hactive_odm  = pipe_param.scale_ratio_depth.hscl_ratio * hactive_odm;
+			full_src_vp_width = pipe_param->scale_ratio_depth.hscl_ratio * pipe_param->dest.full_recout_width;
+			src_hactive_odm  = pipe_param->scale_ratio_depth.hscl_ratio * hactive_odm;
 		}
 
 		if (access_dir == 0) {
@@ -815,7 +815,7 @@ static void get_surf_rq_param(struct display_mode_lib *mode_lib,
 	rq_sizing_param->meta_chunk_bytes = 2048;
 	rq_sizing_param->min_meta_chunk_bytes = 256;
 
-	if (pipe_param.src.hostvm)
+	if (pipe_param->src.hostvm)
 		rq_sizing_param->mpte_group_bytes = 512;
 	else
 		rq_sizing_param->mpte_group_bytes = 2048;
@@ -828,28 +828,28 @@ static void get_surf_rq_param(struct display_mode_lib *mode_lib,
 		vp_height,
 		data_pitch,
 		meta_pitch,
-		pipe_param.src.source_format,
-		pipe_param.src.sw_mode,
-		pipe_param.src.macro_tile_size,
-		pipe_param.src.source_scan,
-		pipe_param.src.hostvm,
+		pipe_param->src.source_format,
+		pipe_param->src.sw_mode,
+		pipe_param->src.macro_tile_size,
+		pipe_param->src.source_scan,
+		pipe_param->src.hostvm,
 		is_chroma,
 		surface_height);
 }
 
 static void dml_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib,
 	display_rq_params_st *rq_param,
-	const display_pipe_params_st pipe_param)
+	const display_pipe_params_st *pipe_param)
 {
 	// get param for luma surface
-	rq_param->yuv420 = pipe_param.src.source_format == dm_420_8
-	|| pipe_param.src.source_format == dm_420_10
-	|| pipe_param.src.source_format == dm_rgbe_alpha
-	|| pipe_param.src.source_format == dm_420_12;
+	rq_param->yuv420 = pipe_param->src.source_format == dm_420_8
+	|| pipe_param->src.source_format == dm_420_10
+	|| pipe_param->src.source_format == dm_rgbe_alpha
+	|| pipe_param->src.source_format == dm_420_12;
 
-	rq_param->yuv420_10bpc = pipe_param.src.source_format == dm_420_10;
+	rq_param->yuv420_10bpc = pipe_param->src.source_format == dm_420_10;
 
-	rq_param->rgbe_alpha = (pipe_param.src.source_format == dm_rgbe_alpha)?1:0;
+	rq_param->rgbe_alpha = (pipe_param->src.source_format == dm_rgbe_alpha)?1:0;
 
 	get_surf_rq_param(mode_lib,
 		&(rq_param->sizing.rq_l),
@@ -859,7 +859,7 @@ static void dml_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib,
 		0,
 		0);
 
-	if (is_dual_plane((enum source_format_class)(pipe_param.src.source_format))) {
+	if (is_dual_plane((enum source_format_class)(pipe_param->src.source_format))) {
 		// get param for chroma surface
 		get_surf_rq_param(mode_lib,
 			&(rq_param->sizing.rq_c),
@@ -871,13 +871,13 @@ static void dml_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib,
 	}
 
 	// calculate how to split the det buffer space between luma and chroma
-	handle_det_buf_split(mode_lib, rq_param, pipe_param.src);
+	handle_det_buf_split(mode_lib, rq_param, pipe_param->src);
 	print__rq_params_st(mode_lib, *rq_param);
 }
 
 void dml30_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 	display_rq_regs_st *rq_regs,
-	const display_pipe_params_st pipe_param)
+	const display_pipe_params_st *pipe_param)
 {
 	display_rq_params_st rq_param = { 0 };
 
@@ -1831,7 +1831,7 @@ static void dml_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib,
 void dml30_rq_dlg_get_dlg_reg(struct display_mode_lib *mode_lib,
 	display_dlg_regs_st *dlg_regs,
 	display_ttu_regs_st *ttu_regs,
-	display_e2e_pipe_params_st *e2e_pipe_param,
+	const display_e2e_pipe_params_st *e2e_pipe_param,
 	const unsigned int num_pipes,
 	const unsigned int pipe_idx,
 	const bool cstate_en,
@@ -1866,7 +1866,7 @@ void dml30_rq_dlg_get_dlg_reg(struct display_mode_lib *mode_lib,
 	// system parameter calculation done
 
 	dml_print("DML_DLG: Calculation for pipe[%d] start\n\n", pipe_idx);
-	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, e2e_pipe_param[pipe_idx].pipe);
+	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, &e2e_pipe_param[pipe_idx].pipe);
 	dml_rq_dlg_get_dlg_params(mode_lib,
 		e2e_pipe_param,
 		num_pipes,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.h b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.h
index c04965cceff3..625e41f8d575 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_rq_dlg_calc_30.h
@@ -41,7 +41,7 @@ struct display_mode_lib;
 //            See also: <display_rq_regs_st>
 void dml30_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param);
+		const display_pipe_params_st *pipe_param);
 
 // Function: dml_rq_dlg_get_dlg_reg
 //   Calculate and return DLG and TTU register struct given the system setting
@@ -57,7 +57,7 @@ void dml30_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 void dml30_rq_dlg_get_dlg_reg(struct display_mode_lib             *mode_lib,
 		display_dlg_regs_st          *dlg_regs,
 		display_ttu_regs_st          *ttu_regs,
-		display_e2e_pipe_params_st   *e2e_pipe_param,
+		const display_e2e_pipe_params_st   *e2e_pipe_param,
 		const unsigned int            num_pipes,
 		const unsigned int            pipe_idx,
 		const bool                    cstate_en,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_rq_dlg_calc_31.c b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_rq_dlg_calc_31.c
index c23905bc733a..57bd4e3f8a82 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_rq_dlg_calc_31.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_rq_dlg_calc_31.c
@@ -738,7 +738,7 @@ static void get_surf_rq_param(
 		display_data_rq_sizing_params_st *rq_sizing_param,
 		display_data_rq_dlg_params_st *rq_dlg_param,
 		display_data_rq_misc_params_st *rq_misc_param,
-		const display_pipe_params_st pipe_param,
+		const display_pipe_params_st *pipe_param,
 		bool is_chroma,
 		bool is_alpha)
 {
@@ -752,33 +752,33 @@ static void get_surf_rq_param(
 
 	// FIXME check if ppe apply for both luma and chroma in 422 case
 	if (is_chroma | is_alpha) {
-		vp_width = pipe_param.src.viewport_width_c / ppe;
-		vp_height = pipe_param.src.viewport_height_c;
-		data_pitch = pipe_param.src.data_pitch_c;
-		meta_pitch = pipe_param.src.meta_pitch_c;
-		surface_height = pipe_param.src.surface_height_y / 2.0;
+		vp_width = pipe_param->src.viewport_width_c / ppe;
+		vp_height = pipe_param->src.viewport_height_c;
+		data_pitch = pipe_param->src.data_pitch_c;
+		meta_pitch = pipe_param->src.meta_pitch_c;
+		surface_height = pipe_param->src.surface_height_y / 2.0;
 	} else {
-		vp_width = pipe_param.src.viewport_width / ppe;
-		vp_height = pipe_param.src.viewport_height;
-		data_pitch = pipe_param.src.data_pitch;
-		meta_pitch = pipe_param.src.meta_pitch;
-		surface_height = pipe_param.src.surface_height_y;
+		vp_width = pipe_param->src.viewport_width / ppe;
+		vp_height = pipe_param->src.viewport_height;
+		data_pitch = pipe_param->src.data_pitch;
+		meta_pitch = pipe_param->src.meta_pitch;
+		surface_height = pipe_param->src.surface_height_y;
 	}
 
-	if (pipe_param.dest.odm_combine) {
+	if (pipe_param->dest.odm_combine) {
 		unsigned int access_dir;
 		unsigned int full_src_vp_width;
 		unsigned int hactive_odm;
 		unsigned int src_hactive_odm;
 
-		access_dir = (pipe_param.src.source_scan == dm_vert); // vp access direction: horizontal or vertical accessed
-		hactive_odm = pipe_param.dest.hactive / ((unsigned int) pipe_param.dest.odm_combine * 2);
+		access_dir = (pipe_param->src.source_scan == dm_vert); // vp access direction: horizontal or vertical accessed
+		hactive_odm = pipe_param->dest.hactive / ((unsigned int) pipe_param->dest.odm_combine * 2);
 		if (is_chroma) {
-			full_src_vp_width = pipe_param.scale_ratio_depth.hscl_ratio_c * pipe_param.dest.full_recout_width;
-			src_hactive_odm = pipe_param.scale_ratio_depth.hscl_ratio_c * hactive_odm;
+			full_src_vp_width = pipe_param->scale_ratio_depth.hscl_ratio_c * pipe_param->dest.full_recout_width;
+			src_hactive_odm = pipe_param->scale_ratio_depth.hscl_ratio_c * hactive_odm;
 		} else {
-			full_src_vp_width = pipe_param.scale_ratio_depth.hscl_ratio * pipe_param.dest.full_recout_width;
-			src_hactive_odm = pipe_param.scale_ratio_depth.hscl_ratio * hactive_odm;
+			full_src_vp_width = pipe_param->scale_ratio_depth.hscl_ratio * pipe_param->dest.full_recout_width;
+			src_hactive_odm = pipe_param->scale_ratio_depth.hscl_ratio * hactive_odm;
 		}
 
 		if (access_dir == 0) {
@@ -808,7 +808,7 @@ static void get_surf_rq_param(
 	rq_sizing_param->meta_chunk_bytes = 2048;
 	rq_sizing_param->min_meta_chunk_bytes = 256;
 
-	if (pipe_param.src.hostvm)
+	if (pipe_param->src.hostvm)
 		rq_sizing_param->mpte_group_bytes = 512;
 	else
 		rq_sizing_param->mpte_group_bytes = 2048;
@@ -822,38 +822,38 @@ static void get_surf_rq_param(
 			vp_height,
 			data_pitch,
 			meta_pitch,
-			pipe_param.src.source_format,
-			pipe_param.src.sw_mode,
-			pipe_param.src.macro_tile_size,
-			pipe_param.src.source_scan,
-			pipe_param.src.hostvm,
+			pipe_param->src.source_format,
+			pipe_param->src.sw_mode,
+			pipe_param->src.macro_tile_size,
+			pipe_param->src.source_scan,
+			pipe_param->src.hostvm,
 			is_chroma,
 			surface_height);
 }
 
-static void dml_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib, display_rq_params_st *rq_param, const display_pipe_params_st pipe_param)
+static void dml_rq_dlg_get_rq_params(struct display_mode_lib *mode_lib, display_rq_params_st *rq_param, const display_pipe_params_st *pipe_param)
 {
 	// get param for luma surface
-	rq_param->yuv420 = pipe_param.src.source_format == dm_420_8 || pipe_param.src.source_format == dm_420_10 || pipe_param.src.source_format == dm_rgbe_alpha
-			|| pipe_param.src.source_format == dm_420_12;
+	rq_param->yuv420 = pipe_param->src.source_format == dm_420_8 || pipe_param->src.source_format == dm_420_10 || pipe_param->src.source_format == dm_rgbe_alpha
+			|| pipe_param->src.source_format == dm_420_12;
 
-	rq_param->yuv420_10bpc = pipe_param.src.source_format == dm_420_10;
+	rq_param->yuv420_10bpc = pipe_param->src.source_format == dm_420_10;
 
-	rq_param->rgbe_alpha = (pipe_param.src.source_format == dm_rgbe_alpha) ? 1 : 0;
+	rq_param->rgbe_alpha = (pipe_param->src.source_format == dm_rgbe_alpha) ? 1 : 0;
 
 	get_surf_rq_param(mode_lib, &(rq_param->sizing.rq_l), &(rq_param->dlg.rq_l), &(rq_param->misc.rq_l), pipe_param, 0, 0);
 
-	if (is_dual_plane((enum source_format_class) (pipe_param.src.source_format))) {
+	if (is_dual_plane((enum source_format_class) (pipe_param->src.source_format))) {
 		// get param for chroma surface
 		get_surf_rq_param(mode_lib, &(rq_param->sizing.rq_c), &(rq_param->dlg.rq_c), &(rq_param->misc.rq_c), pipe_param, 1, rq_param->rgbe_alpha);
 	}
 
 	// calculate how to split the det buffer space between luma and chroma
-	handle_det_buf_split(mode_lib, rq_param, pipe_param.src);
+	handle_det_buf_split(mode_lib, rq_param, pipe_param->src);
 	print__rq_params_st(mode_lib, *rq_param);
 }
 
-void dml31_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib, display_rq_regs_st *rq_regs, const display_pipe_params_st pipe_param)
+void dml31_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib, display_rq_regs_st *rq_regs, const display_pipe_params_st *pipe_param)
 {
 	display_rq_params_st rq_param = {0};
 
@@ -1677,7 +1677,7 @@ void dml31_rq_dlg_get_dlg_reg(
 		struct display_mode_lib *mode_lib,
 		display_dlg_regs_st *dlg_regs,
 		display_ttu_regs_st *ttu_regs,
-		display_e2e_pipe_params_st *e2e_pipe_param,
+		const display_e2e_pipe_params_st *e2e_pipe_param,
 		const unsigned int num_pipes,
 		const unsigned int pipe_idx,
 		const bool cstate_en,
@@ -1704,7 +1704,7 @@ void dml31_rq_dlg_get_dlg_reg(
 	// system parameter calculation done
 
 	dml_print("DML_DLG: Calculation for pipe[%d] start\n\n", pipe_idx);
-	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, e2e_pipe_param[pipe_idx].pipe);
+	dml_rq_dlg_get_rq_params(mode_lib, &rq_param, &e2e_pipe_param[pipe_idx].pipe);
 	dml_rq_dlg_get_dlg_params(
 			mode_lib,
 			e2e_pipe_param,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_rq_dlg_calc_31.h b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_rq_dlg_calc_31.h
index adf8518f761f..8ee991351699 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_rq_dlg_calc_31.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_rq_dlg_calc_31.h
@@ -41,7 +41,7 @@ struct display_mode_lib;
 //            See also: <display_rq_regs_st>
 void dml31_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param);
+		const display_pipe_params_st *pipe_param);
 
 // Function: dml_rq_dlg_get_dlg_reg
 //   Calculate and return DLG and TTU register struct given the system setting
@@ -57,7 +57,7 @@ void dml31_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib,
 void dml31_rq_dlg_get_dlg_reg(struct display_mode_lib             *mode_lib,
 		display_dlg_regs_st          *dlg_regs,
 		display_ttu_regs_st          *ttu_regs,
-		display_e2e_pipe_params_st   *e2e_pipe_param,
+		const display_e2e_pipe_params_st *e2e_pipe_param,
 		const unsigned int            num_pipes,
 		const unsigned int            pipe_idx,
 		const bool                    cstate_en,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.h b/drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.h
index d42a0aeca6be..72b1957022aa 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.h
@@ -49,7 +49,7 @@ struct dml_funcs {
 			struct display_mode_lib *mode_lib,
 			display_dlg_regs_st *dlg_regs,
 			display_ttu_regs_st *ttu_regs,
-			display_e2e_pipe_params_st *e2e_pipe_param,
+			const display_e2e_pipe_params_st *e2e_pipe_param,
 			const unsigned int num_pipes,
 			const unsigned int pipe_idx,
 			const bool cstate_en,
@@ -60,7 +60,7 @@ struct dml_funcs {
 	void (*rq_dlg_get_rq_reg)(
 		struct display_mode_lib *mode_lib,
 		display_rq_regs_st *rq_regs,
-		const display_pipe_params_st pipe_param);
+		const display_pipe_params_st *pipe_param);
 	void (*recalculate)(struct display_mode_lib *mode_lib);
 	void (*validate)(struct display_mode_lib *mode_lib);
 };
-- 
2.33.0


[-- Attachment #3: 0002-drm-amd-display-Allocate-structs-needed-by-dcn_bw_ca.patch --]
[-- Type: text/x-patch, Size: 22557 bytes --]

From eece72e18fc9afb237f11077c0c57d82c59b1d1f Mon Sep 17 00:00:00 2001
From: Harry Wentland <harry.wentland@amd.com>
Date: Tue, 7 Sep 2021 20:22:13 -0400
Subject: [PATCH 2/2] drm/amd/display: Allocate structs needed by
 dcn_bw_calc_rq_dlg_ttu in pipe_ctx

[Why & How]
dcn_bw_calc_rq_dlg_ttu uses a stack frame great than 1024. To solve this
we could allocate the rq_param, dlg_sys_param, and input structs
dynamically. Since this function is inside a kernel_fpu_begin()/end()
call we want to avoid memory allocation. Instead it's much
safer to pre-allocate these on the pipe_ctx.

Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 .../gpu/drm/amd/display/dc/calcs/dcn_calcs.c  |  55 +++---
 .../display/dc/dml/dml1_display_rq_dlg_calc.c | 170 +++++++++---------
 .../display/dc/dml/dml1_display_rq_dlg_calc.h |   8 +-
 .../gpu/drm/amd/display/dc/inc/core_types.h   |   3 +
 4 files changed, 121 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
index 0e18df1283b6..e64145b0a32b 100644
--- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
@@ -459,9 +459,9 @@ static void dcn_bw_calc_rq_dlg_ttu(
 	struct _vcs_dpi_display_dlg_regs_st *dlg_regs = &pipe->dlg_regs;
 	struct _vcs_dpi_display_ttu_regs_st *ttu_regs = &pipe->ttu_regs;
 	struct _vcs_dpi_display_rq_regs_st *rq_regs = &pipe->rq_regs;
-	struct _vcs_dpi_display_rq_params_st rq_param = {0};
-	struct _vcs_dpi_display_dlg_sys_params_st dlg_sys_param = {0};
-	struct _vcs_dpi_display_e2e_pipe_params_st input = { { { 0 } } };
+	struct _vcs_dpi_display_rq_params_st *rq_param = &pipe->dml_rq_param;
+	struct _vcs_dpi_display_dlg_sys_params_st *dlg_sys_param = &pipe->dml_dlg_sys_param;
+	struct _vcs_dpi_display_e2e_pipe_params_st *input = &pipe->dml_input;
 	float total_active_bw = 0;
 	float total_prefetch_bw = 0;
 	int total_flip_bytes = 0;
@@ -470,45 +470,48 @@ static void dcn_bw_calc_rq_dlg_ttu(
 	memset(dlg_regs, 0, sizeof(*dlg_regs));
 	memset(ttu_regs, 0, sizeof(*ttu_regs));
 	memset(rq_regs, 0, sizeof(*rq_regs));
+	memset(rq_param, 0, sizeof(*rq_param));
+	memset(dlg_sys_param, 0, sizeof(*dlg_sys_param));
+	memset(input, 0, sizeof(*input));
 
 	for (i = 0; i < number_of_planes; i++) {
 		total_active_bw += v->read_bandwidth[i];
 		total_prefetch_bw += v->prefetch_bandwidth[i];
 		total_flip_bytes += v->total_immediate_flip_bytes[i];
 	}
-	dlg_sys_param.total_flip_bw = v->return_bw - dcn_bw_max2(total_active_bw, total_prefetch_bw);
-	if (dlg_sys_param.total_flip_bw < 0.0)
-		dlg_sys_param.total_flip_bw = 0;
-
-	dlg_sys_param.t_mclk_wm_us = v->dram_clock_change_watermark;
-	dlg_sys_param.t_sr_wm_us = v->stutter_enter_plus_exit_watermark;
-	dlg_sys_param.t_urg_wm_us = v->urgent_watermark;
-	dlg_sys_param.t_extra_us = v->urgent_extra_latency;
-	dlg_sys_param.deepsleep_dcfclk_mhz = v->dcf_clk_deep_sleep;
-	dlg_sys_param.total_flip_bytes = total_flip_bytes;
-
-	pipe_ctx_to_e2e_pipe_params(pipe, &input.pipe);
-	input.clks_cfg.dcfclk_mhz = v->dcfclk;
-	input.clks_cfg.dispclk_mhz = v->dispclk;
-	input.clks_cfg.dppclk_mhz = v->dppclk;
-	input.clks_cfg.refclk_mhz = dc->res_pool->ref_clocks.dchub_ref_clock_inKhz / 1000.0;
-	input.clks_cfg.socclk_mhz = v->socclk;
-	input.clks_cfg.voltage = v->voltage_level;
+	dlg_sys_param->total_flip_bw = v->return_bw - dcn_bw_max2(total_active_bw, total_prefetch_bw);
+	if (dlg_sys_param->total_flip_bw < 0.0)
+		dlg_sys_param->total_flip_bw = 0;
+
+	dlg_sys_param->t_mclk_wm_us = v->dram_clock_change_watermark;
+	dlg_sys_param->t_sr_wm_us = v->stutter_enter_plus_exit_watermark;
+	dlg_sys_param->t_urg_wm_us = v->urgent_watermark;
+	dlg_sys_param->t_extra_us = v->urgent_extra_latency;
+	dlg_sys_param->deepsleep_dcfclk_mhz = v->dcf_clk_deep_sleep;
+	dlg_sys_param->total_flip_bytes = total_flip_bytes;
+
+	pipe_ctx_to_e2e_pipe_params(pipe, &input->pipe);
+	input->clks_cfg.dcfclk_mhz = v->dcfclk;
+	input->clks_cfg.dispclk_mhz = v->dispclk;
+	input->clks_cfg.dppclk_mhz = v->dppclk;
+	input->clks_cfg.refclk_mhz = dc->res_pool->ref_clocks.dchub_ref_clock_inKhz / 1000.0;
+	input->clks_cfg.socclk_mhz = v->socclk;
+	input->clks_cfg.voltage = v->voltage_level;
 //	dc->dml.logger = pool->base.logger;
-	input.dout.output_format = (v->output_format[in_idx] == dcn_bw_420) ? dm_420 : dm_444;
-	input.dout.output_type  = (v->output[in_idx] == dcn_bw_hdmi) ? dm_hdmi : dm_dp;
+	input->dout.output_format = (v->output_format[in_idx] == dcn_bw_420) ? dm_420 : dm_444;
+	input->dout.output_type  = (v->output[in_idx] == dcn_bw_hdmi) ? dm_hdmi : dm_dp;
 	//input[in_idx].dout.output_standard;
 
 	/*todo: soc->sr_enter_plus_exit_time??*/
-	dlg_sys_param.t_srx_delay_us = dc->dcn_ip->dcfclk_cstate_latency / v->dcf_clk_deep_sleep;
+	dlg_sys_param->t_srx_delay_us = dc->dcn_ip->dcfclk_cstate_latency / v->dcf_clk_deep_sleep;
 
-	dml1_rq_dlg_get_rq_params(dml, &rq_param, input.pipe.src);
+	dml1_rq_dlg_get_rq_params(dml, rq_param, input->pipe.src);
 	dml1_extract_rq_regs(dml, rq_regs, rq_param);
 	dml1_rq_dlg_get_dlg_params(
 			dml,
 			dlg_regs,
 			ttu_regs,
-			rq_param.dlg,
+			&rq_param->dlg,
 			dlg_sys_param,
 			input,
 			true,
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.c b/drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.c
index 8f2b1684c231..0561374cdada 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.c
@@ -231,22 +231,22 @@ static void extract_rq_sizing_regs(
 void dml1_extract_rq_regs(
 		struct display_mode_lib *mode_lib,
 		struct _vcs_dpi_display_rq_regs_st *rq_regs,
-		const struct _vcs_dpi_display_rq_params_st rq_param)
+		const struct _vcs_dpi_display_rq_params_st *rq_param)
 {
 	unsigned int detile_buf_size_in_bytes = mode_lib->ip.det_buffer_size_kbytes * 1024;
 	unsigned int detile_buf_plane1_addr = 0;
 
-	extract_rq_sizing_regs(mode_lib, &(rq_regs->rq_regs_l), rq_param.sizing.rq_l);
-	if (rq_param.yuv420)
-		extract_rq_sizing_regs(mode_lib, &(rq_regs->rq_regs_c), rq_param.sizing.rq_c);
+	extract_rq_sizing_regs(mode_lib, &(rq_regs->rq_regs_l), rq_param->sizing.rq_l);
+	if (rq_param->yuv420)
+		extract_rq_sizing_regs(mode_lib, &(rq_regs->rq_regs_c), rq_param->sizing.rq_c);
 
-	rq_regs->rq_regs_l.swath_height = dml_log2(rq_param.dlg.rq_l.swath_height);
-	rq_regs->rq_regs_c.swath_height = dml_log2(rq_param.dlg.rq_c.swath_height);
+	rq_regs->rq_regs_l.swath_height = dml_log2(rq_param->dlg.rq_l.swath_height);
+	rq_regs->rq_regs_c.swath_height = dml_log2(rq_param->dlg.rq_c.swath_height);
 
 	/* TODO: take the max between luma, chroma chunk size?
 	 * okay for now, as we are setting chunk_bytes to 8kb anyways
 	 */
-	if (rq_param.sizing.rq_l.chunk_bytes >= 32 * 1024) { /*32kb */
+	if (rq_param->sizing.rq_l.chunk_bytes >= 32 * 1024) { /*32kb */
 		rq_regs->drq_expansion_mode = 0;
 	} else {
 		rq_regs->drq_expansion_mode = 2;
@@ -255,9 +255,9 @@ void dml1_extract_rq_regs(
 	rq_regs->mrq_expansion_mode = 1;
 	rq_regs->crq_expansion_mode = 1;
 
-	if (rq_param.yuv420) {
-		if ((double) rq_param.misc.rq_l.stored_swath_bytes
-				/ (double) rq_param.misc.rq_c.stored_swath_bytes <= 1.5) {
+	if (rq_param->yuv420) {
+		if ((double) rq_param->misc.rq_l.stored_swath_bytes
+				/ (double) rq_param->misc.rq_c.stored_swath_bytes <= 1.5) {
 			detile_buf_plane1_addr = (detile_buf_size_in_bytes / 2.0 / 64.0); /* half to chroma */
 		} else {
 			detile_buf_plane1_addr = dml_round_to_multiple(
@@ -998,26 +998,26 @@ void dml1_rq_dlg_get_dlg_params(
 		struct display_mode_lib *mode_lib,
 		struct _vcs_dpi_display_dlg_regs_st *disp_dlg_regs,
 		struct _vcs_dpi_display_ttu_regs_st *disp_ttu_regs,
-		const struct _vcs_dpi_display_rq_dlg_params_st rq_dlg_param,
-		const struct _vcs_dpi_display_dlg_sys_params_st dlg_sys_param,
-		const struct _vcs_dpi_display_e2e_pipe_params_st e2e_pipe_param,
+		const struct _vcs_dpi_display_rq_dlg_params_st *rq_dlg_param,
+		const struct _vcs_dpi_display_dlg_sys_params_st *dlg_sys_param,
+		const struct _vcs_dpi_display_e2e_pipe_params_st *e2e_pipe_param,
 		const bool cstate_en,
 		const bool pstate_en,
 		const bool vm_en,
 		const bool iflip_en)
 {
 	/* Timing */
-	unsigned int htotal = e2e_pipe_param.pipe.dest.htotal;
-	unsigned int hblank_end = e2e_pipe_param.pipe.dest.hblank_end;
-	unsigned int vblank_start = e2e_pipe_param.pipe.dest.vblank_start;
-	unsigned int vblank_end = e2e_pipe_param.pipe.dest.vblank_end;
-	bool interlaced = e2e_pipe_param.pipe.dest.interlaced;
+	unsigned int htotal = e2e_pipe_param->pipe.dest.htotal;
+	unsigned int hblank_end = e2e_pipe_param->pipe.dest.hblank_end;
+	unsigned int vblank_start = e2e_pipe_param->pipe.dest.vblank_start;
+	unsigned int vblank_end = e2e_pipe_param->pipe.dest.vblank_end;
+	bool interlaced = e2e_pipe_param->pipe.dest.interlaced;
 	unsigned int min_vblank = mode_lib->ip.min_vblank_lines;
 
-	double pclk_freq_in_mhz = e2e_pipe_param.pipe.dest.pixel_rate_mhz;
-	double refclk_freq_in_mhz = e2e_pipe_param.clks_cfg.refclk_mhz;
-	double dppclk_freq_in_mhz = e2e_pipe_param.clks_cfg.dppclk_mhz;
-	double dispclk_freq_in_mhz = e2e_pipe_param.clks_cfg.dispclk_mhz;
+	double pclk_freq_in_mhz = e2e_pipe_param->pipe.dest.pixel_rate_mhz;
+	double refclk_freq_in_mhz = e2e_pipe_param->clks_cfg.refclk_mhz;
+	double dppclk_freq_in_mhz = e2e_pipe_param->clks_cfg.dppclk_mhz;
+	double dispclk_freq_in_mhz = e2e_pipe_param->clks_cfg.dispclk_mhz;
 
 	double ref_freq_to_pix_freq;
 	double prefetch_xy_calc_in_dcfclk;
@@ -1160,13 +1160,13 @@ void dml1_rq_dlg_get_dlg_params(
 	disp_dlg_regs->dlg_vblank_end = interlaced ? (vblank_end / 2) : vblank_end; /* 15 bits */
 
 	prefetch_xy_calc_in_dcfclk = 24.0; /* TODO: ip_param */
-	min_dcfclk_mhz = dlg_sys_param.deepsleep_dcfclk_mhz;
+	min_dcfclk_mhz = dlg_sys_param->deepsleep_dcfclk_mhz;
 	t_calc_us = prefetch_xy_calc_in_dcfclk / min_dcfclk_mhz;
-	min_ttu_vblank = dlg_sys_param.t_urg_wm_us;
+	min_ttu_vblank = dlg_sys_param->t_urg_wm_us;
 	if (cstate_en)
-		min_ttu_vblank = dml_max(dlg_sys_param.t_sr_wm_us, min_ttu_vblank);
+		min_ttu_vblank = dml_max(dlg_sys_param->t_sr_wm_us, min_ttu_vblank);
 	if (pstate_en)
-		min_ttu_vblank = dml_max(dlg_sys_param.t_mclk_wm_us, min_ttu_vblank);
+		min_ttu_vblank = dml_max(dlg_sys_param->t_mclk_wm_us, min_ttu_vblank);
 	min_ttu_vblank = min_ttu_vblank + t_calc_us;
 
 	min_dst_y_ttu_vblank = min_ttu_vblank * pclk_freq_in_mhz / (double) htotal;
@@ -1197,59 +1197,59 @@ void dml1_rq_dlg_get_dlg_params(
 	/* ------------------------- */
 	/* Prefetch Calc */
 	/* Source */
-	dcc_en = e2e_pipe_param.pipe.src.dcc;
+	dcc_en = e2e_pipe_param->pipe.src.dcc;
 	dual_plane = is_dual_plane(
-			(enum source_format_class) e2e_pipe_param.pipe.src.source_format);
+			(enum source_format_class) e2e_pipe_param->pipe.src.source_format);
 	mode_422 = 0; /* TODO */
-	access_dir = (e2e_pipe_param.pipe.src.source_scan == dm_vert); /* vp access direction: horizontal or vertical accessed */
+	access_dir = (e2e_pipe_param->pipe.src.source_scan == dm_vert); /* vp access direction: horizontal or vertical accessed */
 	bytes_per_element_l = get_bytes_per_element(
-			(enum source_format_class) e2e_pipe_param.pipe.src.source_format,
+			(enum source_format_class) e2e_pipe_param->pipe.src.source_format,
 			0);
 	bytes_per_element_c = get_bytes_per_element(
-			(enum source_format_class) e2e_pipe_param.pipe.src.source_format,
+			(enum source_format_class) e2e_pipe_param->pipe.src.source_format,
 			1);
-	vp_height_l = e2e_pipe_param.pipe.src.viewport_height;
-	vp_width_l = e2e_pipe_param.pipe.src.viewport_width;
-	vp_height_c = e2e_pipe_param.pipe.src.viewport_height_c;
-	vp_width_c = e2e_pipe_param.pipe.src.viewport_width_c;
+	vp_height_l = e2e_pipe_param->pipe.src.viewport_height;
+	vp_width_l = e2e_pipe_param->pipe.src.viewport_width;
+	vp_height_c = e2e_pipe_param->pipe.src.viewport_height_c;
+	vp_width_c = e2e_pipe_param->pipe.src.viewport_width_c;
 
 	/* Scaling */
-	htaps_l = e2e_pipe_param.pipe.scale_taps.htaps;
-	htaps_c = e2e_pipe_param.pipe.scale_taps.htaps_c;
-	hratios_l = e2e_pipe_param.pipe.scale_ratio_depth.hscl_ratio;
-	hratios_c = e2e_pipe_param.pipe.scale_ratio_depth.hscl_ratio_c;
-	vratio_l = e2e_pipe_param.pipe.scale_ratio_depth.vscl_ratio;
-	vratio_c = e2e_pipe_param.pipe.scale_ratio_depth.vscl_ratio_c;
+	htaps_l = e2e_pipe_param->pipe.scale_taps.htaps;
+	htaps_c = e2e_pipe_param->pipe.scale_taps.htaps_c;
+	hratios_l = e2e_pipe_param->pipe.scale_ratio_depth.hscl_ratio;
+	hratios_c = e2e_pipe_param->pipe.scale_ratio_depth.hscl_ratio_c;
+	vratio_l = e2e_pipe_param->pipe.scale_ratio_depth.vscl_ratio;
+	vratio_c = e2e_pipe_param->pipe.scale_ratio_depth.vscl_ratio_c;
 
 	line_time_in_us = (htotal / pclk_freq_in_mhz);
-	vinit_l = e2e_pipe_param.pipe.scale_ratio_depth.vinit;
-	vinit_c = e2e_pipe_param.pipe.scale_ratio_depth.vinit_c;
-	vinit_bot_l = e2e_pipe_param.pipe.scale_ratio_depth.vinit_bot;
-	vinit_bot_c = e2e_pipe_param.pipe.scale_ratio_depth.vinit_bot_c;
-
-	swath_height_l = rq_dlg_param.rq_l.swath_height;
-	swath_width_ub_l = rq_dlg_param.rq_l.swath_width_ub;
-	dpte_bytes_per_row_ub_l = rq_dlg_param.rq_l.dpte_bytes_per_row_ub;
-	dpte_groups_per_row_ub_l = rq_dlg_param.rq_l.dpte_groups_per_row_ub;
-	meta_pte_bytes_per_frame_ub_l = rq_dlg_param.rq_l.meta_pte_bytes_per_frame_ub;
-	meta_bytes_per_row_ub_l = rq_dlg_param.rq_l.meta_bytes_per_row_ub;
-
-	swath_height_c = rq_dlg_param.rq_c.swath_height;
-	swath_width_ub_c = rq_dlg_param.rq_c.swath_width_ub;
-	dpte_bytes_per_row_ub_c = rq_dlg_param.rq_c.dpte_bytes_per_row_ub;
-	dpte_groups_per_row_ub_c = rq_dlg_param.rq_c.dpte_groups_per_row_ub;
-
-	meta_chunks_per_row_ub_l = rq_dlg_param.rq_l.meta_chunks_per_row_ub;
-	vupdate_offset = e2e_pipe_param.pipe.dest.vupdate_offset;
-	vupdate_width = e2e_pipe_param.pipe.dest.vupdate_width;
-	vready_offset = e2e_pipe_param.pipe.dest.vready_offset;
+	vinit_l = e2e_pipe_param->pipe.scale_ratio_depth.vinit;
+	vinit_c = e2e_pipe_param->pipe.scale_ratio_depth.vinit_c;
+	vinit_bot_l = e2e_pipe_param->pipe.scale_ratio_depth.vinit_bot;
+	vinit_bot_c = e2e_pipe_param->pipe.scale_ratio_depth.vinit_bot_c;
+
+	swath_height_l = rq_dlg_param->rq_l.swath_height;
+	swath_width_ub_l = rq_dlg_param->rq_l.swath_width_ub;
+	dpte_bytes_per_row_ub_l = rq_dlg_param->rq_l.dpte_bytes_per_row_ub;
+	dpte_groups_per_row_ub_l = rq_dlg_param->rq_l.dpte_groups_per_row_ub;
+	meta_pte_bytes_per_frame_ub_l = rq_dlg_param->rq_l.meta_pte_bytes_per_frame_ub;
+	meta_bytes_per_row_ub_l = rq_dlg_param->rq_l.meta_bytes_per_row_ub;
+
+	swath_height_c = rq_dlg_param->rq_c.swath_height;
+	swath_width_ub_c = rq_dlg_param->rq_c.swath_width_ub;
+	dpte_bytes_per_row_ub_c = rq_dlg_param->rq_c.dpte_bytes_per_row_ub;
+	dpte_groups_per_row_ub_c = rq_dlg_param->rq_c.dpte_groups_per_row_ub;
+
+	meta_chunks_per_row_ub_l = rq_dlg_param->rq_l.meta_chunks_per_row_ub;
+	vupdate_offset = e2e_pipe_param->pipe.dest.vupdate_offset;
+	vupdate_width = e2e_pipe_param->pipe.dest.vupdate_width;
+	vready_offset = e2e_pipe_param->pipe.dest.vready_offset;
 
 	dppclk_delay_subtotal = mode_lib->ip.dppclk_delay_subtotal;
 	dispclk_delay_subtotal = mode_lib->ip.dispclk_delay_subtotal;
 	pixel_rate_delay_subtotal = dppclk_delay_subtotal * pclk_freq_in_mhz / dppclk_freq_in_mhz
 			+ dispclk_delay_subtotal * pclk_freq_in_mhz / dispclk_freq_in_mhz;
 
-	vstartup_start = e2e_pipe_param.pipe.dest.vstartup_start;
+	vstartup_start = e2e_pipe_param->pipe.dest.vstartup_start;
 
 	if (interlaced)
 		vstartup_start = vstartup_start / 2;
@@ -1276,13 +1276,13 @@ void dml1_rq_dlg_get_dlg_params(
 	dst_x_after_scaler = 0;
 	dst_y_after_scaler = 0;
 
-	if (e2e_pipe_param.pipe.src.is_hsplit)
+	if (e2e_pipe_param->pipe.src.is_hsplit)
 		dst_x_after_scaler = pixel_rate_delay_subtotal
-				+ e2e_pipe_param.pipe.dest.recout_width;
+				+ e2e_pipe_param->pipe.dest.recout_width;
 	else
 		dst_x_after_scaler = pixel_rate_delay_subtotal;
 
-	if (e2e_pipe_param.dout.output_format == dm_420)
+	if (e2e_pipe_param->dout.output_format == dm_420)
 		dst_y_after_scaler = 1;
 	else
 		dst_y_after_scaler = 0;
@@ -1334,7 +1334,7 @@ void dml1_rq_dlg_get_dlg_params(
 	DTRACE(
 			"DLG: %s: t_srx_delay_us     = %3.2f",
 			__func__,
-			(double) dlg_sys_param.t_srx_delay_us);
+			(double) dlg_sys_param->t_srx_delay_us);
 	DTRACE("DLG: %s: line_time_in_us    = %3.2f", __func__, (double) line_time_in_us);
 	DTRACE("DLG: %s: vupdate_offset     = %d", __func__, vupdate_offset);
 	DTRACE("DLG: %s: vupdate_width      = %d", __func__, vupdate_width);
@@ -1408,12 +1408,12 @@ void dml1_rq_dlg_get_dlg_params(
 	DTRACE("DLG: %s: dpte_row_bytes          = %d", __func__, dpte_row_bytes);
 
 	prefetch_bw = (vm_bytes + 2 * dpte_row_bytes + 2 * meta_row_bytes + sw_bytes) / t_pre_us;
-	flip_bw = ((vm_bytes + dpte_row_bytes + meta_row_bytes) * dlg_sys_param.total_flip_bw)
-			/ (double) dlg_sys_param.total_flip_bytes;
+	flip_bw = ((vm_bytes + dpte_row_bytes + meta_row_bytes) * dlg_sys_param->total_flip_bw)
+			/ (double) dlg_sys_param->total_flip_bytes;
 	t_vm_us = line_time_in_us / 4.0;
 	if (vm_en && dcc_en) {
 		t_vm_us = dml_max(
-				dlg_sys_param.t_extra_us,
+				dlg_sys_param->t_extra_us,
 				dml_max((double) vm_bytes / prefetch_bw, t_vm_us));
 
 		if (iflip_en && !dual_plane) {
@@ -1423,12 +1423,12 @@ void dml1_rq_dlg_get_dlg_params(
 		}
 	}
 
-	t_r0_us = dml_max(dlg_sys_param.t_extra_us - t_vm_us, line_time_in_us - t_vm_us);
+	t_r0_us = dml_max(dlg_sys_param->t_extra_us - t_vm_us, line_time_in_us - t_vm_us);
 
 	if (vm_en || dcc_en) {
 		t_r0_us = dml_max(
 				(double) (dpte_row_bytes + meta_row_bytes) / prefetch_bw,
-				dlg_sys_param.t_extra_us);
+				dlg_sys_param->t_extra_us);
 		t_r0_us = dml_max((double) (line_time_in_us - t_vm_us), t_r0_us);
 
 		if (iflip_en && !dual_plane) {
@@ -1550,15 +1550,15 @@ void dml1_rq_dlg_get_dlg_params(
 			disp_dlg_regs->refcyc_per_meta_chunk_vblank_l;/* dcc for 4:2:0 is not supported in dcn1.0.  assigned to be the same as _l for now */
 
 	/* Active */
-	req_per_swath_ub_l = rq_dlg_param.rq_l.req_per_swath_ub;
-	req_per_swath_ub_c = rq_dlg_param.rq_c.req_per_swath_ub;
-	meta_row_height_l = rq_dlg_param.rq_l.meta_row_height;
+	req_per_swath_ub_l = rq_dlg_param->rq_l.req_per_swath_ub;
+	req_per_swath_ub_c = rq_dlg_param->rq_c.req_per_swath_ub;
+	meta_row_height_l = rq_dlg_param->rq_l.meta_row_height;
 	swath_width_pixels_ub_l = 0;
 	swath_width_pixels_ub_c = 0;
 	scaler_rec_in_width_l = 0;
 	scaler_rec_in_width_c = 0;
-	dpte_row_height_l = rq_dlg_param.rq_l.dpte_row_height;
-	dpte_row_height_c = rq_dlg_param.rq_c.dpte_row_height;
+	dpte_row_height_l = rq_dlg_param->rq_l.dpte_row_height;
+	dpte_row_height_c = rq_dlg_param->rq_c.dpte_row_height;
 
 	disp_dlg_regs->dst_y_per_pte_row_nom_l = (unsigned int) ((double) dpte_row_height_l
 			/ (double) vratio_l * dml_pow(2, 2));
@@ -1650,14 +1650,14 @@ void dml1_rq_dlg_get_dlg_params(
 	refcyc_per_req_delivery_cur0 = 0.;
 
 	full_recout_width = 0;
-	if (e2e_pipe_param.pipe.src.is_hsplit) {
-		if (e2e_pipe_param.pipe.dest.full_recout_width == 0) {
+	if (e2e_pipe_param->pipe.src.is_hsplit) {
+		if (e2e_pipe_param->pipe.dest.full_recout_width == 0) {
 			DTRACE("DLG: %s: Warningfull_recout_width not set in hsplit mode", __func__);
-			full_recout_width = e2e_pipe_param.pipe.dest.recout_width * 2; /* assume half split for dcn1 */
+			full_recout_width = e2e_pipe_param->pipe.dest.recout_width * 2; /* assume half split for dcn1 */
 		} else
-			full_recout_width = e2e_pipe_param.pipe.dest.full_recout_width;
+			full_recout_width = e2e_pipe_param->pipe.dest.full_recout_width;
 	} else
-		full_recout_width = e2e_pipe_param.pipe.dest.recout_width;
+		full_recout_width = e2e_pipe_param->pipe.dest.recout_width;
 
 	refcyc_per_line_delivery_pre_l = get_refcyc_per_delivery(
 			mode_lib,
@@ -1824,9 +1824,9 @@ void dml1_rq_dlg_get_dlg_params(
 	}
 
 	/* TTU - Cursor */
-	hratios_cur0 = e2e_pipe_param.pipe.scale_ratio_depth.hscl_ratio;
-	cur0_src_width = e2e_pipe_param.pipe.src.cur0_src_width; /* cursor source width */
-	cur0_bpp = (enum cursor_bpp) e2e_pipe_param.pipe.src.cur0_bpp;
+	hratios_cur0 = e2e_pipe_param->pipe.scale_ratio_depth.hscl_ratio;
+	cur0_src_width = e2e_pipe_param->pipe.src.cur0_src_width; /* cursor source width */
+	cur0_bpp = (enum cursor_bpp) e2e_pipe_param->pipe.src.cur0_bpp;
 	cur0_req_size = 0;
 	cur0_req_width = 0;
 	cur0_width_ub = 0.0;
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.h b/drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.h
index 9c06913ad767..23d4f30826ff 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.h
+++ b/drivers/gpu/drm/amd/display/dc/dml/dml1_display_rq_dlg_calc.h
@@ -33,7 +33,7 @@ struct display_mode_lib;
 void dml1_extract_rq_regs(
 		struct display_mode_lib *mode_lib,
 		struct _vcs_dpi_display_rq_regs_st *rq_regs,
-		const struct _vcs_dpi_display_rq_params_st rq_param);
+		const struct _vcs_dpi_display_rq_params_st *rq_param);
 /* Function: dml_rq_dlg_get_rq_params
  *  Calculate requestor related parameters that register definition agnostic
  *  (i.e. this layer does try to separate real values from register definition)
@@ -55,9 +55,9 @@ void dml1_rq_dlg_get_dlg_params(
 		struct display_mode_lib *mode_lib,
 		struct _vcs_dpi_display_dlg_regs_st *dlg_regs,
 		struct _vcs_dpi_display_ttu_regs_st *ttu_regs,
-		const struct _vcs_dpi_display_rq_dlg_params_st rq_dlg_param,
-		const struct _vcs_dpi_display_dlg_sys_params_st dlg_sys_param,
-		const struct _vcs_dpi_display_e2e_pipe_params_st e2e_pipe_param,
+		const struct _vcs_dpi_display_rq_dlg_params_st *rq_dlg_param,
+		const struct _vcs_dpi_display_dlg_sys_params_st *dlg_sys_param,
+		const struct _vcs_dpi_display_e2e_pipe_params_st *e2e_pipe_param,
 		const bool cstate_en,
 		const bool pstate_en,
 		const bool vm_en,
diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
index 45a6216dfa2a..4dca14b598dd 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
@@ -366,6 +366,9 @@ struct pipe_ctx {
 	struct _vcs_dpi_display_ttu_regs_st ttu_regs;
 	struct _vcs_dpi_display_rq_regs_st rq_regs;
 	struct _vcs_dpi_display_pipe_dest_params_st pipe_dlg_param;
+	struct _vcs_dpi_display_rq_params_st dml_rq_param;
+	struct _vcs_dpi_display_dlg_sys_params_st dml_dlg_sys_param;
+	struct _vcs_dpi_display_e2e_pipe_params_st dml_input;
 	int det_buffer_size_kb;
 	bool unbounded_req;
 #endif
-- 
2.33.0

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Tue, Sep 7, 2021 at 8:52 PM Harry Wentland <harry.wentland@amd.com> wrote:
>
> Attached patches fix these x86_64 ones reported by Nick:

Hmm.

You didn't seem to fix up the calling convention for print__xyz(),
which still take those xyz structs as pass-by-value.

Obviously it would be good to do things incrementally, so if that
attached patch was just [1/N] I won't complain..

> I'm also seeing one more that might be more challenging to fix but is nearly at 1024:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:3397:6: error: stack frame size of 1064 bytes in function 'dml21_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than=]

Oh Gods, that function is truly something else..

Is there some reason why it's one humongous function, with the
occasional single-line comment?

Because it really looks to me like pretty much everywhere I see one of
those rare comments, I would go "this part should be a function of its
own", and then there would be one caller fuynction that just calls
each of those sub-functions one after the other.

That would - I think - make the code easier to read, and then it would
also make it very obvious where it magically uses a lot of stack.

My suspicion is actually "nowhere". The stack use is just hugely
spread out, and the compiler has just kept accumulating more spill
variables on the frame with no single big reason.

Yes, I see a couple of local structures:

                Pipe myPipe;
                HostVM myHostVM;

but more than that I see several function calls that have basically 62
arguments. And I wish I was making that number up. I'm not. That
"CalculatePrefetchSchedule()" call literally has 62 arguments.

But *all* of the top-level loops in that function literally look like
they could - and should - be functions in their own right. Some of
them would be fairly complex even so (ie that code under the comment

        //Prefetch Check

would be quite the big function all of its own.

We have a coding style thing:

    Documentation/process/coding-style.rst

that says that you should strive to have functions that are "short and
sweet" and fit on one or two screenfuls of text.

That one function from hell is 1832 lines of code.

It really could be improved upon.

               Linus

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Harry Wentland]

On 2021-09-08 12:41 a.m., Linus Torvalds wrote:
> On Tue, Sep 7, 2021 at 8:52 PM Harry Wentland <harry.wentland@amd.com> wrote:
>>
>> Attached patches fix these x86_64 ones reported by Nick:
> 
> Hmm.
> 
> You didn't seem to fix up the calling convention for print__xyz(),
> which still take those xyz structs as pass-by-value.
> 
> Obviously it would be good to do things incrementally, so if that
> attached patch was just [1/N] I won't complain..
> 

You're right. I was focussed on the stack frame limit but fixed up
the rest as well now and sent the series out.

https://lkml.org/lkml/2021/9/8/933

>> I'm also seeing one more that might be more challenging to fix but is nearly at 1024:
>>
>> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.c:3397:6: error: stack frame size of 1064 bytes in function 'dml21_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than=]
> 
> Oh Gods, that function is truly something else..
> 
> Is there some reason why it's one humongous function, with the
> occasional single-line comment?
> 
> Because it really looks to me like pretty much everywhere I see one of
> those rare comments, I would go "this part should be a function of its
> own", and then there would be one caller fuynction that just calls
> each of those sub-functions one after the other.
> 

Yeah, that's what I'm thinking as well. It would likely fix the stack
size, even without dynamically allocating the two structs you mention
below.

> That would - I think - make the code easier to read, and then it would
> also make it very obvious where it magically uses a lot of stack.
> 
> My suspicion is actually "nowhere". The stack use is just hugely
> spread out, and the compiler has just kept accumulating more spill
> variables on the frame with no single big reason.
> 
> Yes, I see a couple of local structures:
> 
>                 Pipe myPipe;
>                 HostVM myHostVM;
> 
> but more than that I see several function calls that have basically 62
> arguments. And I wish I was making that number up. I'm not. That
> "CalculatePrefetchSchedule()" call literally has 62 arguments.
> 
> But *all* of the top-level loops in that function literally look like
> they could - and should - be functions in their own right. Some of
> them would be fairly complex even so (ie that code under the comment
> 
>         //Prefetch Check
> 
> would be quite the big function all of its own.
> 
> We have a coding style thing:
> 
>     Documentation/process/coding-style.rst
> 
> that says that you should strive to have functions that are "short and
> sweet" and fit on one or two screenfuls of text.
> 
> That one function from hell is 1832 lines of code.
> 
> It really could be improved upon.
> 

Absolutely.

The file comes with an (easy to miss) disclaimer that it is
"gcc-parseable HW gospel":
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_mode_vba_20.c?h=v5.14#n32

The display_mode_vba* stuff deals with bandwidth formulas from HW
designers. At some point in the past we attempted to convert them
to something more readable and elegant but would often run into
difficulties getting support from the right people when things
wouldn't work. Using the HW designer's code directly tends to
short circuit any arguments about SW correctness.

In short, I don't really like this code but it works. It helps
prevent black screens and underflows on the display.

We try to follow the coding-style.rst for the most part elsewhere,
though there are still plenty of areas where we can improve.

Harry

>                Linus
> 

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/7/21 10:10 AM, Linus Torvalds wrote:
> On Tue, Sep 7, 2021 at 2:11 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>>> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:452:13: error: stack frame size (1800) exceeds limit (1280) in function 'dcn_bw_calc_rq_dlg_ttu' [-Werror,-Wframe-larger-than]
>>> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.c:1657:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml21_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
>>> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.c:1831:6: error: stack frame size (1352) exceeds limit (1280) in function 'dml30_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
>>> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_rq_dlg_calc_31.c:1676:6: error: stack frame size (1336) exceeds limit (1280) in function 'dml31_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
>>> x86_64-alpine.log:drivers/vhost/scsi.c:1831:12: error: stack frame size (1320) exceeds limit (1280) in function 'vhost_scsi_release' [-Werror,-Wframe-larger-than]
>>>

FWIW, the above is because of

static void vhost_scsi_flush(struct vhost_scsi *vs)
{
         struct vhost_scsi_inflight *old_inflight[VHOST_SCSI_MAX_VQ];

where VHOST_SCSI_MAX_VQ=128. Presumably some versions of clang inline
this function. gcc has the same problem here - its stack frame size is
also > 1024 for vhost_scsi_flush().

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Nathan Chancellor]

On 9/7/2021 10:48 AM, Guenter Roeck wrote:
> On 9/7/21 10:10 AM, Linus Torvalds wrote:
>> On Tue, Sep 7, 2021 at 2:11 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>>
>>>> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:452:13: 
>>>> error: stack frame size (1800) exceeds limit (1280) in function 
>>>> 'dcn_bw_calc_rq_dlg_ttu' [-Werror,-Wframe-larger-than]
>>>> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.c:1657:6: 
>>>> error: stack frame size (1336) exceeds limit (1280) in function 
>>>> 'dml21_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
>>>> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_rq_dlg_calc_30.c:1831:6: 
>>>> error: stack frame size (1352) exceeds limit (1280) in function 
>>>> 'dml30_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
>>>> x86_64-alpine.log:drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_rq_dlg_calc_31.c:1676:6: 
>>>> error: stack frame size (1336) exceeds limit (1280) in function 
>>>> 'dml31_rq_dlg_get_dlg_reg' [-Werror,-Wframe-larger-than]
>>>> x86_64-alpine.log:drivers/vhost/scsi.c:1831:12: error: stack frame 
>>>> size (1320) exceeds limit (1280) in function 'vhost_scsi_release' 
>>>> [-Werror,-Wframe-larger-than]
>>>>
> 
> FWIW, the above is because of
> 
> static void vhost_scsi_flush(struct vhost_scsi *vs)
> {
>          struct vhost_scsi_inflight *old_inflight[VHOST_SCSI_MAX_VQ];
> 
> where VHOST_SCSI_MAX_VQ=128. Presumably some versions of clang inline
> this function. gcc has the same problem here - its stack frame size is
> also > 1024 for vhost_scsi_flush().

Good to know. When investigating these, I intend to compare against GCC 
to see what the difference is to know if it is a problem with the code 
or a compiler issue.

Cheers,
Nathan

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Nathan Chancellor]

Hi Arnd,

On Tue, Sep 07, 2021 at 11:11:17AM +0200, Arnd Bergmann wrote:
> On Tue, Sep 7, 2021 at 4:32 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > arm32-allmodconfig.log: crypto/wp512.c:782:13: error: stack frame size (1176) exceeds limit (1024) in function 'wp512_process_buffer' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/firmware/tegra/bpmp-debugfs.c:294:12: error: stack frame size (1256) exceeds limit (1024) in function 'bpmp_debug_show' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/firmware/tegra/bpmp-debugfs.c:357:16: error: stack frame size (1264) exceeds limit (1024) in function 'bpmp_debug_store' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1384) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5560) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/mtd/chips/cfi_cmdset_0001.c:1872:12: error: stack frame size (1064) exceeds limit (1024) in function 'cfi_intelext_writev' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/ntb/hw/idt/ntb_hw_idt.c:1041:27: error: stack frame size (1032) exceeds limit (1024) in function 'idt_scan_mws' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/staging/fbtft/fbtft-core.c:902:12: error: stack frame size (1072) exceeds limit (1024) in function 'fbtft_init_display_from_property' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/staging/fbtft/fbtft-core.c:992:5: error: stack frame size (1064) exceeds limit (1024) in function 'fbtft_init_display' [-Werror,-Wframe-larger-than]
> > arm32-allmodconfig.log: drivers/staging/rtl8723bs/core/rtw_security.c:1288:5: error: stack frame size (1040) exceeds limit (1024) in function 'rtw_aes_decrypt' [-Werror,-Wframe-larger-than]
> > arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1376) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
> > arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5384) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]
> >
> > Aside from the dce_calcs.c warnings, these do not seem too bad. I
> > believe allmodconfig turns on UBSAN but it could also be aggressive
> > inlining by clang. I intend to look at all -Wframe-large-than warnings
> > closely later.
> 
> I've had them close to zero in the past, but a couple of new ones came in.
> 
> The amdgpu ones are probably not fixable unless they stop using 64-bit
> floats in the kernel for
> random calculations. The crypto/* ones tend to be compiler bugs, but hard to fix

I have started taking a look at these. Most of the allmodconfig ones
appear to be related to CONFIG_KASAN, which is now supported for
CONFIG_ARM.

The two in bpmp-debugfs.c appear regardless of CONFIG_KASAN and it turns
out that you actually submitted a patch for these:

https://lore.kernel.org/r/20201204193714.3134651-1-arnd@kernel.org/

Is it worth resending or pinging that?

The dce_calcs.c ones also appear without CONFIG_KASAN, which you noted
is probably unavoidable.

The other ones only appear with CONFIG_KASAN. I have not investigated
each instance to see exactly how much KASAN makes the stack blow up.
Perhaps it is worth setting the default of CONFIG_FRAME_WARN to a higher
value with clang+COMPILE_TEST+KASAN?

> > It appears that both Arch Linux and Fedora define CONFIG_FRAME_WARN
> > as 1024, below its default of 2048. I am not sure these look particurly
> > scary (although there are some that are rather large that need to be
> > looked at).
> 
> For 64-bit, you usually need 1280 bytes stack space to get a
> reasonably clean build,
> anything that uses more than that tends to be a bug in the code but we
> never warned
> about those by default as the default warning limit in defconfig is 2048.
> 
> I think the distros using 1024 did that because they use a common base config
> for 32-bit and 64-bit targets.

That is a fair explanation.

> > I suspect this is a backend problem because these do not really appear
> > in any other configurations (might also be something with a sanitizer?)
> 
> Agreed. Someone needs to bisect the .config or the compiler flags to see what
> triggers them.

For other people following along, there were a lot of
-Wframe-larger-than instances from RISC-V allmodconfig.

Turns out this is because CONFIG_KASAN_STACK is not respected with
RISC-V. They do not set CONFIG_KASAN_SHADOW_OFFSET so following along in
scripts/Makefile.kasan, CFLAGS_KASAN_SHADOW does not get set to
anything, which means that only '-fsanitize=kernel-address' gets added
to the command line, with none of the other parameters.

I guess there are a couple of ways to tackle this:

1. RISC-V could implement CONFIG_KASAN_SHADOW_OFFSET. They mention that
   the logic of KASAN_SHADOW_OFFSET was taken from arm64 but they did
   not borrow the Kconfig logic it seems.

2. asan-stack could be hoisted out of the else branch so that it is
   always enabled/disabled regardless of KASAN_SHADOW_OFFSET being
   defined, which resolved all of these warnings for me in my testing.

I am adding the KASAN and RISC-V folks to CC for this reason.

Cheers,
Nathan

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/8/21 1:55 PM, Nathan Chancellor wrote:
> Hi Arnd,
> 
> On Tue, Sep 07, 2021 at 11:11:17AM +0200, Arnd Bergmann wrote:
>> On Tue, Sep 7, 2021 at 4:32 AM Nathan Chancellor <nathan@kernel.org> wrote:
>>>
>>> arm32-allmodconfig.log: crypto/wp512.c:782:13: error: stack frame size (1176) exceeds limit (1024) in function 'wp512_process_buffer' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/firmware/tegra/bpmp-debugfs.c:294:12: error: stack frame size (1256) exceeds limit (1024) in function 'bpmp_debug_show' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/firmware/tegra/bpmp-debugfs.c:357:16: error: stack frame size (1264) exceeds limit (1024) in function 'bpmp_debug_store' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1384) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5560) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/mtd/chips/cfi_cmdset_0001.c:1872:12: error: stack frame size (1064) exceeds limit (1024) in function 'cfi_intelext_writev' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/ntb/hw/idt/ntb_hw_idt.c:1041:27: error: stack frame size (1032) exceeds limit (1024) in function 'idt_scan_mws' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/staging/fbtft/fbtft-core.c:902:12: error: stack frame size (1072) exceeds limit (1024) in function 'fbtft_init_display_from_property' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/staging/fbtft/fbtft-core.c:992:5: error: stack frame size (1064) exceeds limit (1024) in function 'fbtft_init_display' [-Werror,-Wframe-larger-than]
>>> arm32-allmodconfig.log: drivers/staging/rtl8723bs/core/rtw_security.c:1288:5: error: stack frame size (1040) exceeds limit (1024) in function 'rtw_aes_decrypt' [-Werror,-Wframe-larger-than]
>>> arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1376) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
>>> arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5384) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]
>>>
>>> Aside from the dce_calcs.c warnings, these do not seem too bad. I
>>> believe allmodconfig turns on UBSAN but it could also be aggressive
>>> inlining by clang. I intend to look at all -Wframe-large-than warnings
>>> closely later.
>>
>> I've had them close to zero in the past, but a couple of new ones came in.
>>
>> The amdgpu ones are probably not fixable unless they stop using 64-bit
>> floats in the kernel for
>> random calculations. The crypto/* ones tend to be compiler bugs, but hard to fix
> 
> I have started taking a look at these. Most of the allmodconfig ones
> appear to be related to CONFIG_KASAN, which is now supported for
> CONFIG_ARM.
> 

Would it make sense to make KASAN depend on !COMPILE_TEST ?
After all, the point of KASAN is runtime testing, not build testing.

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Marco Elver]

On Wed, Sep 08, 2021 at 02:16PM -0700, Guenter Roeck wrote:
> On 9/8/21 1:55 PM, Nathan Chancellor wrote:
[...]
> > I have started taking a look at these. Most of the allmodconfig ones
> > appear to be related to CONFIG_KASAN, which is now supported for
> > CONFIG_ARM.
> > 
> 
> Would it make sense to make KASAN depend on !COMPILE_TEST ?
> After all, the point of KASAN is runtime testing, not build testing.

It'd be good to avoid. It has helped uncover build issues with KASAN in
the past. Or at least make it dependent on the problematic architecture.
For example if arm is a problem, something like this:

--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -71,7 +71,7 @@ config ARM
 	select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
 	select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
 	select HAVE_ARCH_KGDB if !CPU_ENDIAN_BE32 && MMU
-	select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL
+	select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL && (!COMPILE_TEST || !CC_IS_CLANG)
 	select HAVE_ARCH_MMAP_RND_BITS if MMU
 	select HAVE_ARCH_PFN_VALID
 	select HAVE_ARCH_SECCOMP

More generally, with clang, the problem is known and due to KASAN stack
instrumentation (CONFIG_KASAN_STACK):

 | config KASAN_STACK
 |         bool "Enable stack instrumentation (unsafe)" if CC_IS_CLANG && !COMPILE_TEST
 |         depends on KASAN_GENERIC || KASAN_SW_TAGS
 |         depends on !ARCH_DISABLE_KASAN_INLINE
 |         default y if CC_IS_GCC
 |         help
 |           The LLVM stack address sanitizer has a know problem that
 |           causes excessive stack usage in a lot of functions, see
 |           https://bugs.llvm.org/show_bug.cgi?id=38809
 |           Disabling asan-stack makes it safe to run kernels build
 |           with clang-8 with KASAN enabled, though it loses some of
 |           the functionality.
 |           This feature is always disabled when compile-testing with clang
 |           to avoid cluttering the output in stack overflow warnings,
 |           but clang users can still enable it for builds without
 |           CONFIG_COMPILE_TEST.  On gcc it is assumed to always be safe
 |           to use and enabled by default.
 |           If the architecture disables inline instrumentation, stack
 |           instrumentation is also disabled as it adds inline-style
 |           instrumentation that is run unconditionally.

This is already disabled if COMPILE_TEST and building with clang. As
far as I know, there's no easy fix for clang and it's been discussed
many times over with LLVM devs.

Thanks,
-- Marco

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Christoph Hellwig]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 949 bytes --]

On Wed, Sep 08, 2021 at 11:58:56PM +0200, Marco Elver wrote:
> It'd be good to avoid. It has helped uncover build issues with KASAN in
> the past. Or at least make it dependent on the problematic architecture.
> For example if arm is a problem, something like this:

I'm also seeing quite a few stack size warnings with KASAN on x86_64
without COMPILT_TEST using gcc 10.2.1 from Debian.  In fact there are a
few warnings without KASAN, but with KASAN there are a lot more.
I'll try to find some time to dig into them.

While we're at it, with -Werror something like this is really futile:

drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function ‘amdgpu_bo_support_uswc’:
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:493:2: warning: #warning
Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance thanks to write-combining [-Wcpp
  493 | #warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
      |  ^~~~~~~

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/8/21 10:58 PM, Christoph Hellwig wrote:
> On Wed, Sep 08, 2021 at 11:58:56PM +0200, Marco Elver wrote:
>> It'd be good to avoid. It has helped uncover build issues with KASAN in
>> the past. Or at least make it dependent on the problematic architecture.
>> For example if arm is a problem, something like this:
> 
> I'm also seeing quite a few stack size warnings with KASAN on x86_64
> without COMPILT_TEST using gcc 10.2.1 from Debian.  In fact there are a
> few warnings without KASAN, but with KASAN there are a lot more.
> I'll try to find some time to dig into them.
> 
> While we're at it, with -Werror something like this is really futile:
> 
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function ‘amdgpu_bo_support_uswc’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:493:2: warning: #warning
> Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance thanks to write-combining [-Wcpp
>    493 | #warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
>        |  ^~~~~~~
> 

I have been wondering if all those #warning "errors" should either
be removed or be replaced with "#pragma message".

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Christian König]

Am 09.09.21 um 08:07 schrieb Guenter Roeck:
> On 9/8/21 10:58 PM, Christoph Hellwig wrote:
>> On Wed, Sep 08, 2021 at 11:58:56PM +0200, Marco Elver wrote:
>>> It'd be good to avoid. It has helped uncover build issues with KASAN in
>>> the past. Or at least make it dependent on the problematic 
>>> architecture.
>>> For example if arm is a problem, something like this:
>>
>> I'm also seeing quite a few stack size warnings with KASAN on x86_64
>> without COMPILT_TEST using gcc 10.2.1 from Debian.  In fact there are a
>> few warnings without KASAN, but with KASAN there are a lot more.
>> I'll try to find some time to dig into them.
>>
>> While we're at it, with -Werror something like this is really futile:
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function 
>> ‘amdgpu_bo_support_uswc’:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:493:2: warning: #warning
>> Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance 
>> thanks to write-combining [-Wcpp
>>    493 | #warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for 
>> better performance \
>>        |  ^~~~~~~

Ah, yes good point!

>
> I have been wondering if all those #warning "errors" should either
> be removed or be replaced with "#pragma message".

Well we started to add those warnings because people compiled their 
kernel with CONFIG_MTRR and CONFIG_X86_PAT and was then wondering why 
the performance of the display driver was so crappy.

When those warning now generate an error which you have to disable 
explicitly then that might not be bad at all.

It at least points people to this setting and makes it really clear that 
they are doing something very unusual and need to keep in mind that it 
might not have the desired result.

Regards,
Christian.

>
> Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Guenter Roeck]

On 9/9/21 12:30 AM, Christian König wrote:
> Am 09.09.21 um 08:07 schrieb Guenter Roeck:
>> On 9/8/21 10:58 PM, Christoph Hellwig wrote:
>>> On Wed, Sep 08, 2021 at 11:58:56PM +0200, Marco Elver wrote:
>>>> It'd be good to avoid. It has helped uncover build issues with KASAN in
>>>> the past. Or at least make it dependent on the problematic architecture.
>>>> For example if arm is a problem, something like this:
>>>
>>> I'm also seeing quite a few stack size warnings with KASAN on x86_64
>>> without COMPILT_TEST using gcc 10.2.1 from Debian.  In fact there are a
>>> few warnings without KASAN, but with KASAN there are a lot more.
>>> I'll try to find some time to dig into them.
>>>
>>> While we're at it, with -Werror something like this is really futile:
>>>
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function ‘amdgpu_bo_support_uswc’:
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:493:2: warning: #warning
>>> Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance thanks to write-combining [-Wcpp
>>>    493 | #warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
>>>        |  ^~~~~~~
> 
> Ah, yes good point!
> 
>>
>> I have been wondering if all those #warning "errors" should either
>> be removed or be replaced with "#pragma message".
> 
> Well we started to add those warnings because people compiled their kernel with CONFIG_MTRR and CONFIG_X86_PAT and was then wondering why the performance of the display driver was so crappy.
> 
> When those warning now generate an error which you have to disable explicitly then that might not be bad at all.
> 
> It at least points people to this setting and makes it really clear that they are doing something very unusual and need to keep in mind that it might not have the desired result.
> 

That specific warning is surrounded with "#ifndef CONFIG_COMPILE_TEST"
so it doesn't really matter because it doesn't cause test build failures.
Of course, we could do the same for any #warning which does now
cause a test build failure.

Guenter

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Marco Elver]

On Thu, 9 Sept 2021 at 07:59, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Sep 08, 2021 at 11:58:56PM +0200, Marco Elver wrote:
> > It'd be good to avoid. It has helped uncover build issues with KASAN in
> > the past. Or at least make it dependent on the problematic architecture.
> > For example if arm is a problem, something like this:
>
> I'm also seeing quite a few stack size warnings with KASAN on x86_64
> without COMPILT_TEST using gcc 10.2.1 from Debian.  In fact there are a
> few warnings without KASAN, but with KASAN there are a lot more.
> I'll try to find some time to dig into them.

Right, this reminded me that we actually at least double the real
stack size for KASAN builds, because it inherently requires more stack
space. I think we need Wframe-larger-than to match that, otherwise
we'll just keep having this problem:

https://lkml.kernel.org/r/20210909104925.809674-1-elver@google.com

> While we're at it, with -Werror something like this is really futile:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c: In function ‘amdgpu_bo_support_uswc’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:493:2: warning: #warning
> Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance thanks to write-combining [-Wcpp
>   493 | #warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
>       |  ^~~~~~~

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Arnd Bergmann]

On Thu, Sep 9, 2021 at 12:54 PM Marco Elver <elver@google.com> wrote:
> On Thu, 9 Sept 2021 at 07:59, Christoph Hellwig <hch@infradead.org> wrote:
> > On Wed, Sep 08, 2021 at 11:58:56PM +0200, Marco Elver wrote:
> > > It'd be good to avoid. It has helped uncover build issues with KASAN in
> > > the past. Or at least make it dependent on the problematic architecture.
> > > For example if arm is a problem, something like this:
> >
> > I'm also seeing quite a few stack size warnings with KASAN on x86_64
> > without COMPILT_TEST using gcc 10.2.1 from Debian.  In fact there are a
> > few warnings without KASAN, but with KASAN there are a lot more.
> > I'll try to find some time to dig into them.
>
> Right, this reminded me that we actually at least double the real
> stack size for KASAN builds, because it inherently requires more stack
> space. I think we need Wframe-larger-than to match that, otherwise
> we'll just keep having this problem:
>
> https://lkml.kernel.org/r/20210909104925.809674-1-elver@google.com

The problem with this is that it completely defeats the point of the
stack size warnings in allmodconfig kernels when they have KASAN
enabled and end up missing obvious code bugs in drivers that put
large structures on the stack. Let's not go there.

        Arnd

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Marco Elver]

On Thu, 9 Sept 2021 at 13:00, Arnd Bergmann <arnd@kernel.org> wrote:
> On Thu, Sep 9, 2021 at 12:54 PM Marco Elver <elver@google.com> wrote:
> > On Thu, 9 Sept 2021 at 07:59, Christoph Hellwig <hch@infradead.org> wrote:
> > > On Wed, Sep 08, 2021 at 11:58:56PM +0200, Marco Elver wrote:
> > > > It'd be good to avoid. It has helped uncover build issues with KASAN in
> > > > the past. Or at least make it dependent on the problematic architecture.
> > > > For example if arm is a problem, something like this:
> > >
> > > I'm also seeing quite a few stack size warnings with KASAN on x86_64
> > > without COMPILT_TEST using gcc 10.2.1 from Debian.  In fact there are a
> > > few warnings without KASAN, but with KASAN there are a lot more.
> > > I'll try to find some time to dig into them.
> >
> > Right, this reminded me that we actually at least double the real
> > stack size for KASAN builds, because it inherently requires more stack
> > space. I think we need Wframe-larger-than to match that, otherwise
> > we'll just keep having this problem:
> >
> > https://lkml.kernel.org/r/20210909104925.809674-1-elver@google.com
>
> The problem with this is that it completely defeats the point of the
> stack size warnings in allmodconfig kernels when they have KASAN
> enabled and end up missing obvious code bugs in drivers that put
> large structures on the stack. Let's not go there.

Sure, but the reality is that the real stack size is already doubled
for KASAN. And that should be reflected in Wframe-larger-than.

Either that, or we just have to live with the occasional warning (that
is likely benign). But with WERROR we're now forced to make the
defaults as sane as possible. If the worry is allmodconfig, maybe we
do have to make KASAN dependent on !COMPILE_TEST, even though that's
not great either because it has caught real issues in the past (it'll
also mean doing the same for all other instrumentation-based tools,
like KCSAN, UBSAN, etc.).

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Arnd Bergmann]

On Thu, Sep 9, 2021 at 1:43 PM Marco Elver <elver@google.com> wrote:
> On Thu, 9 Sept 2021 at 13:00, Arnd Bergmann <arnd@kernel.org> wrote:
> > On Thu, Sep 9, 2021 at 12:54 PM Marco Elver <elver@google.com> wrote:
> > > On Thu, 9 Sept 2021 at 07:59, Christoph Hellwig <hch@infradead.org> wrote:
> > > > On Wed, Sep 08, 2021 at 11:58:56PM +0200, Marco Elver wrote:
> > > > > It'd be good to avoid. It has helped uncover build issues with KASAN in
> > > > > the past. Or at least make it dependent on the problematic architecture.
> > > > > For example if arm is a problem, something like this:
> > > >
> > > > I'm also seeing quite a few stack size warnings with KASAN on x86_64
> > > > without COMPILT_TEST using gcc 10.2.1 from Debian.  In fact there are a
> > > > few warnings without KASAN, but with KASAN there are a lot more.
> > > > I'll try to find some time to dig into them.
> > >
> > > Right, this reminded me that we actually at least double the real
> > > stack size for KASAN builds, because it inherently requires more stack
> > > space. I think we need Wframe-larger-than to match that, otherwise
> > > we'll just keep having this problem:
> > >
> > > https://lkml.kernel.org/r/20210909104925.809674-1-elver@google.com
> >
> > The problem with this is that it completely defeats the point of the
> > stack size warnings in allmodconfig kernels when they have KASAN
> > enabled and end up missing obvious code bugs in drivers that put
> > large structures on the stack. Let's not go there.
>
> Sure, but the reality is that the real stack size is already doubled
> for KASAN. And that should be reflected in Wframe-larger-than.

I don't think "double" is an accurate description of what is going on,
it's much more complex than this. There are some functions
that completely explode with KASAN_STACK enabled on clang,
and many other functions instances that don't grow much at all.

I've been building randconfig kernels for a long time with KASAN_STACK
enabled on gcc, and the limit increased to 1440 bytes for 32-bit
and not increased beyond the normal 2048 bytes for 64-bit. I have
some patches to address the outliers and should go through and
resend some of those.

With the same limits and patches using clang, and KASAN=y but
KASAN_STACK=n I also get no warnings in randconfig builds,
but KASAN_STACK on clang doesn't really seem to have a good
limit that would make an allmodconfig kernel build with no warnings.

These are the worst offenders I see based on configuration, using
an 32-bit ARM allmodconfig with my fixups:

gcc-11, KASAN, no KASAN_STACK, FRAME_WARN=1024:
(nothing)

gcc-11, KASAN_STACK:
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:782:1:
warning: the frame size of 1416 bytes is larger than 1024 bytes
[-Wframe-larger-than=]
drivers/media/dvb-frontends/mxl5xx.c:1575:1: warning: the frame size
of 1240 bytes is larger than 1024 bytes [-Wframe-larger-than=]
drivers/mtd/nftlcore.c:468:1: warning: the frame size of 1232 bytes is
larger than 1024 bytes [-Wframe-larger-than=]
drivers/char/ipmi/ipmi_msghandler.c:4880:1: warning: the frame size of
1232 bytes is larger than 1024 bytes [-Wframe-larger-than=]
drivers/mtd/chips/cfi_cmdset_0001.c:1870:1: warning: the frame size of
1224 bytes is larger than 1024 bytes [-Wframe-larger-than=]
drivers/net/wireless/ath/ath9k/ar9003_paprd.c:749:1: warning: the
frame size of 1216 bytes is larger than 1024 bytes
[-Wframe-larger-than=]
drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:136:1: warning:
the frame size of 1216 bytes is larger than 1024 bytes
[-Wframe-larger-than=]
drivers/ntb/hw/idt/ntb_hw_idt.c:1116:1: warning: the frame size of
1200 bytes is larger than 1024 bytes [-Wframe-larger-than=]
net/dcb/dcbnl.c:1172:1: warning: the frame size of 1192 bytes is
larger than 1024 bytes [-Wframe-larger-than=]
fs/select.c:1042:1: warning: the frame size of 1192 bytes is larger
than 1024 bytes [-Wframe-larger-than=]

clang-12 KASAN, no KASAN_STACK, FRAME_WARN=1024:

kernel/trace/trace_events_hist.c:4601:13: error: stack frame size 1384
exceeds limit 1024 in function 'hist_trigger_print_key'
[-Werror,-Wframe-larger-than]
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3045:6:
error: stack frame size 1384 exceeds limit 1024 in function 'bw_calcs'
[-Werror,-Wframe-larger-than]
drivers/staging/fbtft/fbtft-core.c:992:5: error: stack frame size 1208
exceeds limit 1024 in function 'fbtft_init_display'
[-Werror,-Wframe-larger-than]
crypto/wp512.c:782:13: error: stack frame size 1176 exceeds limit 1024
in function 'wp512_process_buffer' [-Werror,-Wframe-larger-than]
drivers/staging/fbtft/fbtft-core.c:902:12: error: stack frame size
1080 exceeds limit 1024 in function 'fbtft_init_display_from_property'
[-Werror,-Wframe-larger-than]
drivers/mtd/chips/cfi_cmdset_0001.c:1872:12: error: stack frame size
1064 exceeds limit 1024 in function 'cfi_intelext_writev'
[-Werror,-Wframe-larger-than]
drivers/staging/rtl8723bs/core/rtw_security.c:1288:5: error: stack
frame size 1040 exceeds limit 1024 in function 'rtw_aes_decrypt'
[-Werror,-Wframe-larger-than]
drivers/ntb/hw/idt/ntb_hw_idt.c:1041:27: error: stack frame size 1032
exceeds limit 1024 in function 'idt_scan_mws'
[-Werror,-Wframe-larger-than]

clang-12, KASAN_STACK:

drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame
size 20608 exceeds limit 1024 in function 'ocrdma_dbgfs_ops_read'
[-Werror,-Wframe-larger-than]
lib/bitfield_kunit.c:60:20: error: stack frame size 10336 exceeds
limit 10240 in function 'test_bitfields_constants'
[-Werror,-Wframe-larger-than]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:9012:13: error: stack
frame size 9952 exceeds limit 1024 in function 'rt2800_init_rfcsr'
[-Werror,-Wframe-larger-than]
drivers/net/usb/r8152.c:7486:13: error: stack frame size 8768 exceeds
limit 1024 in function 'r8156b_hw_phy_cfg'
[-Werror,-Wframe-larger-than]
drivers/media/dvb-frontends/nxt200x.c:915:12: error: stack frame size
8192 exceeds limit 1024 in function 'nxt2004_init'
[-Werror,-Wframe-larger-than]
drivers/net/wan/slic_ds26522.c:203:12: error: stack frame size 8064
exceeds limit 1024 in function 'slic_ds26522_probe'
[-Werror,-Wframe-larger-than]
drivers/firmware/broadcom/bcm47xx_sprom.c:188:13: error: stack frame
size 8064 exceeds limit 1024 in function 'bcm47xx_sprom_fill_auto'
[-Werror,-Wframe-larger-than]
drivers/media/dvb-frontends/drxd_hard.c:2857:12: error: stack frame
size 7584 exceeds limit 1024 in function 'drxd_set_frontend'
[-Werror,-Wframe-larger-than]
drivers/media/dvb-frontends/nxt200x.c:519:12: error: stack frame size
6848 exceeds limit 1024 in function
'nxt200x_setup_frontend_parameters' [-Werror,-Wframe-larger-than]
drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:17019:13:
error: stack frame size 6560 exceeds limit 1024 in function
'wlc_phy_workarounds_nphy' [-Werror,-Wframe-larger-than]

> Either that, or we just have to live with the occasional warning (that
> is likely benign). But with WERROR we're now forced to make the
> defaults as sane as possible. If the worry is allmodconfig, maybe we
> do have to make KASAN dependent on !COMPILE_TEST, even though that's
> not great either because it has caught real issues in the past (it'll
> also mean doing the same for all other instrumentation-based tools,
> like KCSAN, UBSAN, etc.).

I would prefer going back to marking KASAN_STACK as broken on clang, it does
not seem like the warnings on the symbol were enough to stop people from
attempting to using it, and the remaining warnings seem fixable with a small
increase of the FRAME_WARN when using KASAN with clang but no KASAN_STACK,
or when using KASAN_STACK with gcc.

      Arnd

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Thu, Sep 9, 2021 at 4:43 AM Marco Elver <elver@google.com> wrote:
>
> Sure, but the reality is that the real stack size is already doubled
> for KASAN. And that should be reflected in Wframe-larger-than.

I don't think that's true.

Quite the reverse, in fact.

Yes, the *dynamic* stack size is doubled due to KASAN, because it will
cause much deeper callchains.

But the individual frames don't grow that much apart from compilers
doing stupid things (ie apparently clang and KASAN_STACK), and if
anything, the deeper dynamic call chains means that the individual
frame size being small is even *more* important, but we do compensate
for the deeper stacks by making THREAD_SIZE_ORDER bigger at least on
x86.

Honestly, I am not even happy with the current "2048 bytes for
64-bit". The excuse has been that 64-bit needs more stack, but all it
ever did was clearly to just allow people to just do bad things.

Because a 1kB stack frame is horrendous even in 64-bit. That's not
"spill some registers" kind of stack frame. That's "put a big
structure on the stack" kind of stack frame regardless of any other
issues.

And no, "but we have 16kB of stack and we'll switch stacks on
interrupts" is not an excuse for one single level to use up 1kB, much
less 2kB.  Does anybody seriously believe that we don't quite normally
have stacks that are easily tens of frames deep?

Without having some true "this is the full callchain" information, the
best we can do is just limit individual stack frames. And 2kB is
*excessive*.

                     Linus

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Linus Torvalds]

On Wed, Sep 8, 2021 at 10:59 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> While we're at it, with -Werror something like this is really futile:

Yeah, I'm thinking we could do

 -Wno-error=cpp

to at least allow the cpp warnings to come through without being fatal.

Because while they can be annoying too, they are most definitely under
our direct control, so..

I didn't actually test that, but I think it should work.

That said, maybe they should just be removed. They might be better off
just as Kconfig rules, rather than as a "hey, you screwed up your
Kconfig" warning after the fact.

             Linus

Re: [PATCH] Enable '-Werror' by default for all kernel builds

[Arnd Bergmann]

On Wed, Sep 8, 2021 at 10:55 PM Nathan Chancellor <nathan@kernel.org> wrote:
> On Tue, Sep 07, 2021 at 11:11:17AM +0200, Arnd Bergmann wrote:
> > On Tue, Sep 7, 2021 at 4:32 AM Nathan Chancellor <nathan@kernel.org> wrote:
function 'rtw_aes_decrypt' [-Werror,-Wframe-larger-than]
> > > arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3043:6: error: stack frame size (1376) exceeds limit (1024) in function 'bw_calcs' [-Werror,-Wframe-larger-than]
> > > arm32-fedora.log: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:77:13: error: stack frame size (5384) exceeds limit (1024) in function 'calculate_bandwidth' [-Werror,-Wframe-larger-than]
> > >
> > > Aside from the dce_calcs.c warnings, these do not seem too bad. I
> > > believe allmodconfig turns on UBSAN but it could also be aggressive
> > > inlining by clang. I intend to look at all -Wframe-large-than warnings
> > > closely later.
> >
> > I've had them close to zero in the past, but a couple of new ones came in.
> >
> > The amdgpu ones are probably not fixable unless they stop using 64-bit
> > floats in the kernel for
> > random calculations. The crypto/* ones tend to be compiler bugs, but hard to fix
>
> I have started taking a look at these. Most of the allmodconfig ones
> appear to be related to CONFIG_KASAN, which is now supported for
> CONFIG_ARM.
>
> The two in bpmp-debugfs.c appear regardless of CONFIG_KASAN and it turns
> out that you actually submitted a patch for these:
>
> https://lore.kernel.org/r/20201204193714.3134651-1-arnd@kernel.org/
>
> Is it worth resending or pinging that?

I'm now restarting from a clean tree for my randconfig patches to see which
ones are actually needed, will hopefully get to that.

> The dce_calcs.c ones also appear without CONFIG_KASAN, which you noted
> is probably unavoidable.

(adding amdgpu folks to Cc here)

Harry Wentland did a nice rework for dcn_calcs.c that should also be
portable to dce_calcs.c, I hope that he will be able to get to that as well.

Looking at my older patches now, I found that I had only suppressed that one
and given up fixing it, but I did put my analysis into
https://bugs.llvm.org/show_bug.cgi?id=42551, which should be helpful
for addressing it in either the kernel or the compiler.

        Arnd