[PATCH] mtd: cfi_cmdset_0001: shrink do_write_buffer() stack frame

Karl Mehltretter <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.infradead.lists.linux-mtd
Message-ID <[email protected]>
arm32 allmodconfig fails to build with gcc:

  drivers/mtd/chips/cfi_cmdset_0001.c:1883:1: error: the frame size of 1296
  bytes is larger than 1280 bytes [-Werror=frame-larger-than=]

With MTD_MAP_BANK_WIDTH_32 a map_word is 32 bytes, and with KASAN_STACK
every temporary gets its own redzoned slot.  do_write_buffer() builds a
map_word for each of the twelve commands it issues, and those temporaries
dominate its frame.

Issue the commands through a small noinline helper that takes the
command as a plain value and builds the map_word in its own frame.  The
sequence of reads and writes to the chip is unchanged.

The helper must not be inlined, and reusing one map_word local is not
enough: clang cannot assume the callee does not observe the local, so it
still returns each CMD() into a per-call-site temporary, and the frame
grows (1760 -> 1824 bytes in cfi_intelext_writev(), into which clang
inlines do_write_buffer()).

Frame sizes with MTD_MAP_BANK_WIDTH_32 and KASAN_STACK:

  gcc 15.2   do_write_buffer()      1296 -> 520
  clang 21   cfi_intelext_writev()  1760 -> 896

The helper itself takes 168 (gcc) / 160 (clang) bytes.

Reported-by: Andy Shevchenko <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Suggested-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <[email protected]>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 42 ++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index b73596a8e021..a7c77c4e76ae 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -1716,12 +1716,24 @@ static int cfi_intelext_write_words (struct mtd_info *mtd, loff_t to , size_t le
 }
 
 
+/*
+ * Keep noinline: inlined, the map_word temporaries put do_write_buffer() over
+ * the frame-size limit with MTD_MAP_BANK_WIDTH_32 and KASAN_STACK.
+ */
+static noinline void __xipram cfi_write_cmd(struct map_info *map,
+					    unsigned long cmd, unsigned long adr)
+{
+	struct cfi_private *cfi = map->fldrv_priv;
+
+	map_write(map, CMD(cmd), adr);
+}
+
 static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 				    unsigned long adr, const struct kvec **pvec,
 				    unsigned long *pvec_seek, int len)
 {
 	struct cfi_private *cfi = map->fldrv_priv;
-	map_word status, write_cmd, datum;
+	map_word status, datum;
 	unsigned long cmd_adr;
 	int ret, wbufsize, word_gap, words;
 	const struct kvec *vec;
@@ -1740,9 +1752,6 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	if (is_LH28F640BF(cfi))
 		cmd_adr = adr;
 
-	/* Let's determine this according to the interleave only once */
-	write_cmd = (cfi->cfiq->P_ID != P_ID_INTEL_PERFORMANCE) ? CMD(0xe8) : CMD(0xe9);
-
 	mutex_lock(&chip->mutex);
 	ret = get_chip(map, chip, cmd_adr, FL_WRITING);
 	if (ret) {
@@ -1759,7 +1768,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	   So we must check here and reset those bits if they're set. Otherwise
 	   we're just pissing in the wind */
 	if (chip->state != FL_STATUS) {
-		map_write(map, CMD(0x70), cmd_adr);
+		cfi_write_cmd(map, 0x70, cmd_adr);
 		chip->state = FL_STATUS;
 	}
 	status = map_read(map, cmd_adr);
@@ -1767,21 +1776,22 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 		xip_enable(map, chip, cmd_adr);
 		printk(KERN_WARNING "SR.4 or SR.5 bits set in buffer write (status %lx). Clearing.\n", status.x[0]);
 		xip_disable(map, chip, cmd_adr);
-		map_write(map, CMD(0x50), cmd_adr);
-		map_write(map, CMD(0x70), cmd_adr);
+		cfi_write_cmd(map, 0x50, cmd_adr);
+		cfi_write_cmd(map, 0x70, cmd_adr);
 	}
 
 	chip->state = FL_WRITING_TO_BUFFER;
-	map_write(map, write_cmd, cmd_adr);
+	cfi_write_cmd(map, (cfi->cfiq->P_ID != P_ID_INTEL_PERFORMANCE) ? 0xe8 : 0xe9,
+		      cmd_adr);
 	ret = WAIT_TIMEOUT(map, chip, cmd_adr, 0, 0);
 	if (ret) {
 		/* Argh. Not ready for write to buffer */
 		map_word Xstatus = map_read(map, cmd_adr);
-		map_write(map, CMD(0x70), cmd_adr);
+		cfi_write_cmd(map, 0x70, cmd_adr);
 		chip->state = FL_STATUS;
 		status = map_read(map, cmd_adr);
-		map_write(map, CMD(0x50), cmd_adr);
-		map_write(map, CMD(0x70), cmd_adr);
+		cfi_write_cmd(map, 0x50, cmd_adr);
+		cfi_write_cmd(map, 0x70, cmd_adr);
 		xip_enable(map, chip, cmd_adr);
 		printk(KERN_ERR "%s: Chip not ready for buffer write. Xstatus = %lx, status = %lx\n",
 				map->name, Xstatus.x[0], status.x[0]);
@@ -1800,7 +1810,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	}
 
 	/* Write length of data to come */
-	map_write(map, CMD(words), cmd_adr );
+	cfi_write_cmd(map, words, cmd_adr);
 
 	/* Write data */
 	vec = *pvec;
@@ -1837,7 +1847,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	*pvec_seek = vec_seek;
 
 	/* GO GO GO */
-	map_write(map, CMD(0xd0), cmd_adr);
+	cfi_write_cmd(map, 0xd0, cmd_adr);
 	chip->state = FL_WRITING;
 
 	ret = INVAL_CACHE_AND_WAIT(map, chip, cmd_adr,
@@ -1845,7 +1855,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 				   chip->buffer_write_time,
 				   chip->buffer_write_time_max);
 	if (ret) {
-		map_write(map, CMD(0x70), cmd_adr);
+		cfi_write_cmd(map, 0x70, cmd_adr);
 		chip->state = FL_STATUS;
 		xip_enable(map, chip, cmd_adr);
 		printk(KERN_ERR "%s: buffer write error (status timeout)\n", map->name);
@@ -1858,8 +1868,8 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 		unsigned long chipstatus = MERGESTATUS(status);
 
 		/* reset status */
-		map_write(map, CMD(0x50), cmd_adr);
-		map_write(map, CMD(0x70), cmd_adr);
+		cfi_write_cmd(map, 0x50, cmd_adr);
+		cfi_write_cmd(map, 0x70, cmd_adr);
 		xip_enable(map, chip, cmd_adr);
 
 		if (chipstatus & 0x02) {
-- 
2.39.5 (Apple Git-154)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.