linux-kernel - Re: [PATCH] drm: a6xx: avoid excessive stack usage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241019093146.kdp25pir5onjmg4g@hu-akhilpo-hyd.qualcomm.com>
Date: Sat, 19 Oct 2024 15:01:46 +0530
From: Akhil P Oommen <quic_akhilpo@...cinc.com>
To: Arnd Bergmann <arnd@...nel.org>
CC: Rob Clark <robdclark@...il.com>,
        Abhinav Kumar
	<quic_abhinavk@...cinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        "Nathan
 Chancellor" <nathan@...nel.org>,
        Arnd Bergmann <arnd@...db.de>, Sean Paul
	<sean@...rly.run>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Marijn Suijten
	<marijn.suijten@...ainline.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Bill Wendling <morbo@...gle.com>,
        Justin Stitt <justinstitt@...gle.com>, <linux-arm-msm@...r.kernel.org>,
        <dri-devel@...ts.freedesktop.org>, <freedreno@...ts.freedesktop.org>,
        <linux-kernel@...r.kernel.org>, <llvm@...ts.linux.dev>
Subject: Re: [PATCH] drm: a6xx: avoid excessive stack usage

On Fri, Oct 18, 2024 at 03:11:38PM +0000, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> Clang-19 and above sometimes end up with multiple copies of the large
> a6xx_hfi_msg_bw_table structure on the stack. The problem is that
> a6xx_hfi_send_bw_table() calls a number of device specific functions to
> fill the structure, but these create another copy of the structure on
> the stack which gets copied to the first.
> 
> If the functions get inlined, that busts the warning limit:
> 
> drivers/gpu/drm/msm/adreno/a6xx_hfi.c:631:12: error: stack frame size (1032) exceeds limit (1024) in 'a6xx_hfi_send_bw_table' [-Werror,-Wframe-larger-than]

Why does this warning says that the limit is 1024? 1024 bytes is too small, isn't it?

-Akhil.

> 
> Mark all of them as 'noinline_for_stack' ensure we only have one copy
> of the structure per function.
> 
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> index cdb3f6e74d3e..5699e0420eb8 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -259,7 +259,8 @@ static int a6xx_hfi_send_perf_table(struct a6xx_gmu *gmu)
>  		NULL, 0);
>  }
>  
> -static void a618_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +/* noinline to avoid having multiple copies of 'msg' on stack */
> +static noinline_for_stack void a618_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	/* Send a single "off" entry since the 618 GMU doesn't do bus scaling */
>  	msg->bw_level_num = 1;
> @@ -287,7 +288,7 @@ static void a618_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][0] =  0x60000001;
>  }
>  
> -static void a619_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a619_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	msg->bw_level_num = 13;
>  
> @@ -346,7 +347,7 @@ static void a619_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[0][0] = 0x40000000;
>  }
>  
> -static void a640_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a640_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	/*
>  	 * Send a single "off" entry just to get things running
> @@ -385,7 +386,7 @@ static void a640_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][2] =  0x60000001;
>  }
>  
> -static void a650_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a650_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	/*
>  	 * Send a single "off" entry just to get things running
> @@ -416,7 +417,7 @@ static void a650_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][0] =  0x60000001;
>  }
>  
> -static void a690_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a690_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	/*
>  	 * Send a single "off" entry just to get things running
> @@ -447,7 +448,7 @@ static void a690_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][0] =  0x60000001;
>  }
>  
> -static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	/*
>  	 * Send a single "off" entry just to get things running
> @@ -478,7 +479,7 @@ static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][0] =  0x60000001;
>  }
>  
> -static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	/*
>  	 * Send a single "off" entry just to get things running
> @@ -509,7 +510,7 @@ static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][0] =  0x60000001;
>  }
>  
> -static void a730_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a730_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	msg->bw_level_num = 12;
>  
> @@ -565,7 +566,7 @@ static void a730_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][0] = 0x60000001;
>  }
>  
> -static void a740_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a740_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	msg->bw_level_num = 1;
>  
> @@ -590,7 +591,7 @@ static void a740_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  	msg->cnoc_cmds_data[1][0] = 0x60000001;
>  }
>  
> -static void a6xx_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
> +static noinline_for_stack void a6xx_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
>  {
>  	/* Send a single "off" entry since the 630 GMU doesn't do bus scaling */
>  	msg->bw_level_num = 1;
> -- 
> 2.39.5
>