-----Original Message-----
From: Kalle Valo [mailto:kvalo@qca.qualcomm.com]
Sent: Tuesday, November 26, 2013 6:40 PM
To: Liu Hui-R64343
Cc: linux-arm-kernel@lists.infradead.org; linville@tuxdriver.com; linux-
wireless@vger.kernel.org; netdev@vger.kernel.org; linux-
kernel@vger.kernel.org; ath6kl-devel@qca.qualcomm.com
Subject: Re: [PATCH] ath6kl: sdio: fix system panic when doing wifi
stress test

Hi Jason,

Jason Liu [off-list ref] writes:

quoted

When did the wifi iperf test, meet one following kernel panic:
command: iperf -c $TARGET_IP -i 5 -t 50 -w 1M

Unable to handle kernel paging request at virtual address 1a480000 pgd
= 80004000 [1a480000] *pgd=00000000 Internal error: Oops: 805 [#1] SMP
ARM

[...]

quoted

The kernel panic is caused by the sg_buf is not set correctly with the
following code when compiled with Yocto GCC 4.8.1:

drivers/net/wireless/ath/ath6kl/hif.h:
struct hif_scatter_req {
        struct list_head list;
        /* address for the read/write operation */
        u32 addr;
	...

        /* bounce buffer for upper layers to copy to/from */
        u8 *virt_dma_buf;

        struct hif_scatter_item scat_list[1];

        u32 scat_q_depth;
};

(Note: the scat_req.scat_list[] will dynamiclly grow with run-time)

There's actually a major bug right there, scat_list can corrupt
scat_q_depth.

quoted

The GCC 4.8.1 compiler will not do the for-loop till scat_entries,
instead, it only run one round loop. This may be caused by that the
GCC 4.8.1 thought that the scat_list only have one item and then no
need to do full iteration, but this is simply wrong by looking at the
assebly code. This will cause the sg buffer not get set when

scat_entries > 1 and thus lead to kernel panic.

quoted

This patch is a workaround to the GCC 4.8.1 complier issue by passing
the entry address of the scat_req->scat_list to the for-loop and
interate it, then, GCC 4.8.1 will do the full for-loop correctly.
(Note: This issue not observed with GCC 4.7.2, only found on the GCC
4.8.1)

This patch does not change any function logic and no any performance

downgrade.

[...]

quoted

+	scat_list = &scat_req->scat_list[0];
+
 	/* assemble SG list */
-	for (i = 0; i < scat_req->scat_entries; i++, sg++) {
+	for (i = 0; i < scat_req->scat_entries; i++, sg++, scat_list++) {
 		ath6kl_dbg(ATH6KL_DBG_SCATTER, "%d: addr:0x%p, len:%d\n",
-			   i, scat_req->scat_list[i].buf,
-			   scat_req->scat_list[i].len);
+			   i, scat_list->buf, scat_list->len);

-		sg_set_buf(sg, scat_req->scat_list[i].buf,
-			   scat_req->scat_list[i].len);
+		sg_set_buf(sg, scat_list->buf, scat_list->len);
 	}

Working around the problem by adding a temporary variable makes me a bit
worried, I would rather fix the root cause. Is the root cause by that we
define the field with scat_list[1]?

Yes, this is what I assumed. 

Does the patch below help? It would also fix the corruption with
scat_q_depth. Please note that I have only compile tested it. And I might
have also missed something important, so please review it carefully.

Yes, Firstly, I have looked at the asm code and the compiler(gcc 4.8.1) works correctly after applying
the following patch. Secondly, I have tested the patch with compiler(gcc 4.8.1) on the real HW, and it
works fine too. Without the patch, the kernel crash will happen 100%.

Thus, for the patch:

Acked-by: Jason Liu <redacted>
Tested-by: Jason Liu <redacted>

Jason Liu