* [PATCH] firmware: cs_dsp: avoid large local variables
@ 2024-12-16 8:33 Arnd Bergmann
2024-12-16 10:38 ` Richard Fitzgerald
0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2024-12-16 8:33 UTC (permalink / raw)
To: Simon Trimmer, Charles Keepax, Richard Fitzgerald, Mark Brown
Cc: Arnd Bergmann, patches, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
Having 1280 bytes of local variables on the stack exceeds the limit
on 32-bit architectures:
drivers/firmware/cirrus/test/cs_dsp_test_bin.c: In function 'bin_patch_mixed_packed_unpacked_random':
drivers/firmware/cirrus/test/cs_dsp_test_bin.c:2097:1: error: the frame size of 1784 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Use dynamic allocation for the largest two here.
Fixes: dd0b6b1f29b9 ("firmware: cs_dsp: Add KUnit testing of bin file download")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
.../firmware/cirrus/test/cs_dsp_test_bin.c | 36 +++++++++++--------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/firmware/cirrus/test/cs_dsp_test_bin.c b/drivers/firmware/cirrus/test/cs_dsp_test_bin.c
index 689190453307..f1955813919c 100644
--- a/drivers/firmware/cirrus/test/cs_dsp_test_bin.c
+++ b/drivers/firmware/cirrus/test/cs_dsp_test_bin.c
@@ -1978,8 +1978,10 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
4, 51, 76, 72, 16, 6, 39, 62, 15, 41, 28, 73, 53, 40, 45, 54,
14, 55, 46, 66, 64, 59, 23, 9, 67, 47, 19, 71, 35, 18, 42, 1,
};
- u32 packed_payload[80][3];
- u32 unpacked_payload[80];
+ struct {
+ u32 packed[80][3];
+ u32 unpacked[80];
+ } *payload;
u32 readback[3];
unsigned int alg_base_words, patch_pos_words;
unsigned int alg_base_in_packed_regs, patch_pos_in_packed_regs;
@@ -1988,8 +1990,12 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
struct firmware *fw;
int i;
- get_random_bytes(packed_payload, sizeof(packed_payload));
- get_random_bytes(unpacked_payload, sizeof(unpacked_payload));
+ payload = kmalloc(sizeof(*payload), GFP_KERNEL);
+ if (!payload)
+ return;
+
+ get_random_bytes(payload->packed, sizeof(payload->packed));
+ get_random_bytes(payload->unpacked, sizeof(payload->unpacked));
/* Create a patch entry for every offset in offset_words[] */
for (i = 0; i < ARRAY_SIZE(offset_words); ++i) {
@@ -2010,8 +2016,8 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
bin_test_mock_algs[0].ver,
param->mem_type,
payload_offset,
- packed_payload[i],
- sizeof(packed_payload[i]));
+ payload->packed[i],
+ sizeof(payload->packed[i]));
} else {
payload_offset = offset_words[i] * 4;
cs_dsp_mock_bin_add_patch(priv->local->bin_builder,
@@ -2019,8 +2025,8 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
bin_test_mock_algs[0].ver,
unpacked_mem_type,
payload_offset,
- &unpacked_payload[i],
- sizeof(unpacked_payload[i]));
+ &payload->unpacked[i],
+ sizeof(payload->unpacked[i]));
}
}
@@ -2033,7 +2039,7 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
/*
* Readback the packed registers that should have been written.
* Place the values into the expected location in readback[] so
- * that the content of readback[] should match packed_payload[]
+ * that the content of readback[] should match payload->packed[]
*/
for (i = 0; i < ARRAY_SIZE(offset_words); ++i) {
alg_base_words = cs_dsp_mock_xm_header_get_alg_base_in_words(priv,
@@ -2055,16 +2061,16 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
regmap_raw_read(priv->dsp->regmap, reg_addr, readback,
sizeof(readback)),
0);
- KUNIT_EXPECT_MEMEQ(test, readback, packed_payload[i], sizeof(packed_payload[i]));
+ KUNIT_EXPECT_MEMEQ(test, readback, payload->packed[i], sizeof(payload->packed[i]));
/* Drop expected writes from the cache */
- cs_dsp_mock_regmap_drop_bytes(priv, reg_addr, sizeof(packed_payload[i]));
+ cs_dsp_mock_regmap_drop_bytes(priv, reg_addr, sizeof(payload->packed[i]));
}
/*
* Readback the unpacked registers that should have been written.
* Place the values into the expected location in readback[] so
- * that the content of readback[] should match unpacked_payload[]
+ * that the content of readback[] should match payload->unpacked[]
*/
for (i = 0; i < ARRAY_SIZE(offset_words); ++i) {
alg_base_words = cs_dsp_mock_xm_header_get_alg_base_in_words(priv,
@@ -2085,15 +2091,17 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
regmap_raw_read(priv->dsp->regmap, reg_addr,
&readback[0], sizeof(readback[0])),
0);
- KUNIT_EXPECT_EQ(test, readback[0], unpacked_payload[i]);
+ KUNIT_EXPECT_EQ(test, readback[0], payload->unpacked[i]);
/* Drop expected writes from the cache */
- cs_dsp_mock_regmap_drop_bytes(priv, reg_addr, sizeof(unpacked_payload[i]));
+ cs_dsp_mock_regmap_drop_bytes(priv, reg_addr, sizeof(payload->unpacked[i]));
}
/* Drop expected writes and the cache should then be clean */
cs_dsp_mock_xm_header_drop_from_regmap_cache(priv);
KUNIT_EXPECT_FALSE(test, cs_dsp_mock_regmap_is_dirty(priv, true));
+
+ kfree(payload);
}
/* Bin file with name and multiple info blocks */
--
2.39.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] firmware: cs_dsp: avoid large local variables
2024-12-16 8:33 [PATCH] firmware: cs_dsp: avoid large local variables Arnd Bergmann
@ 2024-12-16 10:38 ` Richard Fitzgerald
2024-12-16 11:18 ` Richard Fitzgerald
0 siblings, 1 reply; 3+ messages in thread
From: Richard Fitzgerald @ 2024-12-16 10:38 UTC (permalink / raw)
To: Arnd Bergmann, Simon Trimmer, Charles Keepax, Mark Brown
Cc: Arnd Bergmann, patches, linux-kernel
On 16/12/2024 8:33 am, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Having 1280 bytes of local variables on the stack exceeds the limit
> on 32-bit architectures:
>
> drivers/firmware/cirrus/test/cs_dsp_test_bin.c: In function 'bin_patch_mixed_packed_unpacked_random':
> drivers/firmware/cirrus/test/cs_dsp_test_bin.c:2097:1: error: the frame size of 1784 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> Use dynamic allocation for the largest two here.
>
> Fixes: dd0b6b1f29b9 ("firmware: cs_dsp: Add KUnit testing of bin file download")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> .../firmware/cirrus/test/cs_dsp_test_bin.c | 36 +++++++++++--------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/firmware/cirrus/test/cs_dsp_test_bin.c b/drivers/firmware/cirrus/test/cs_dsp_test_bin.c
> index 689190453307..f1955813919c 100644
> --- a/drivers/firmware/cirrus/test/cs_dsp_test_bin.c
> +++ b/drivers/firmware/cirrus/test/cs_dsp_test_bin.c
> @@ -1978,8 +1978,10 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
> 4, 51, 76, 72, 16, 6, 39, 62, 15, 41, 28, 73, 53, 40, 45, 54,
> 14, 55, 46, 66, 64, 59, 23, 9, 67, 47, 19, 71, 35, 18, 42, 1,
> };
> - u32 packed_payload[80][3];
> - u32 unpacked_payload[80];
> + struct {
> + u32 packed[80][3];
> + u32 unpacked[80];
> + } *payload;
> u32 readback[3];
> unsigned int alg_base_words, patch_pos_words;
> unsigned int alg_base_in_packed_regs, patch_pos_in_packed_regs;
> @@ -1988,8 +1990,12 @@ static void bin_patch_mixed_packed_unpacked_random(struct kunit *test)
> struct firmware *fw;
> int i;
>
> - get_random_bytes(packed_payload, sizeof(packed_payload));
> - get_random_bytes(unpacked_payload, sizeof(unpacked_payload));
> + payload = kmalloc(sizeof(*payload), GFP_KERNEL);
Should be kunit_kmalloc() so the kunit framework frees it automatically
if any test case fails and the test terminates early.
Apart from that it looks ok to me.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] firmware: cs_dsp: avoid large local variables
2024-12-16 10:38 ` Richard Fitzgerald
@ 2024-12-16 11:18 ` Richard Fitzgerald
0 siblings, 0 replies; 3+ messages in thread
From: Richard Fitzgerald @ 2024-12-16 11:18 UTC (permalink / raw)
To: Arnd Bergmann, Simon Trimmer, Charles Keepax, Mark Brown
Cc: Arnd Bergmann, patches, linux-kernel
On 16/12/2024 10:38 am, Richard Fitzgerald wrote:
> On 16/12/2024 8:33 am, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> Having 1280 bytes of local variables on the stack exceeds the limit
>> on 32-bit architectures:
>>
>> drivers/firmware/cirrus/test/cs_dsp_test_bin.c: In function
>> 'bin_patch_mixed_packed_unpacked_random':
>> drivers/firmware/cirrus/test/cs_dsp_test_bin.c:2097:1: error: the
>> frame size of 1784 bytes is larger than 1024 bytes [-Werror=frame-
>> larger-than=]
>>
>> Use dynamic allocation for the largest two here.
>>
>> Fixes: dd0b6b1f29b9 ("firmware: cs_dsp: Add KUnit testing of bin file
>> download")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> .../firmware/cirrus/test/cs_dsp_test_bin.c | 36 +++++++++++--------
>> 1 file changed, 22 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/firmware/cirrus/test/cs_dsp_test_bin.c b/drivers/
>> firmware/cirrus/test/cs_dsp_test_bin.c
>> index 689190453307..f1955813919c 100644
>> --- a/drivers/firmware/cirrus/test/cs_dsp_test_bin.c
>> +++ b/drivers/firmware/cirrus/test/cs_dsp_test_bin.c
>> @@ -1978,8 +1978,10 @@ static void
>> bin_patch_mixed_packed_unpacked_random(struct kunit *test)
>> 4, 51, 76, 72, 16, 6, 39, 62, 15, 41, 28, 73, 53, 40, 45, 54,
>> 14, 55, 46, 66, 64, 59, 23, 9, 67, 47, 19, 71, 35, 18, 42, 1,
>> };
>> - u32 packed_payload[80][3];
>> - u32 unpacked_payload[80];
>> + struct {
>> + u32 packed[80][3];
>> + u32 unpacked[80];
>> + } *payload;
>> u32 readback[3];
>> unsigned int alg_base_words, patch_pos_words;
>> unsigned int alg_base_in_packed_regs, patch_pos_in_packed_regs;
>> @@ -1988,8 +1990,12 @@ static void
>> bin_patch_mixed_packed_unpacked_random(struct kunit *test)
>> struct firmware *fw;
>> int i;
>> - get_random_bytes(packed_payload, sizeof(packed_payload));
>> - get_random_bytes(unpacked_payload, sizeof(unpacked_payload));
>> + payload = kmalloc(sizeof(*payload), GFP_KERNEL);
>
> Should be kunit_kmalloc() so the kunit framework frees it automatically
> if any test case fails and the test terminates early.
>
> Apart from that it looks ok to me.
Also the check for NULL should use KUNIT_ASSERT_NOT_NULL() so the test
case will error-out.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-12-16 11:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-16 8:33 [PATCH] firmware: cs_dsp: avoid large local variables Arnd Bergmann
2024-12-16 10:38 ` Richard Fitzgerald
2024-12-16 11:18 ` Richard Fitzgerald
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox