diff mbox series

[RFC] Revert "ath10k: fix DMA related firmware crashes on multiple devices"

Message ID 1571734629-18028-1-git-send-email-zhichen@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series [RFC] Revert "ath10k: fix DMA related firmware crashes on multiple devices" | expand

Commit Message

Zhi Chen Oct. 22, 2019, 8:57 a.m. UTC
This reverts commit 76d164f582150fd0259ec0fcbc485470bcd8033e.
PCIe hung issue was observed on multiple platforms. The issue was reproduced
when DUT was configured as AP and associated with 50+ STAs.

With PCIe protocol analyzer, we can see DMA Read crossing 4KB boundary when
issue happened. It broke PCIe spec and caused PCIe stuck.

Tested:  IPQ8064 + QCA9984 with firmware 10.4-3.10-00047
         QCS404 + QCA9984 with firmware 10.4-3.9.0.2--00044
         Synaptics AS370 + QCA9888  with firmware 10.4-3.9.0.2--00040

Signed-off-by: Zhi Chen <zhichen@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/hw.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Tom Psyborg Oct. 22, 2019, 10:07 a.m. UTC | #1
On 22/10/2019, Zhi Chen <zhichen@codeaurora.org> wrote:
> This reverts commit 76d164f582150fd0259ec0fcbc485470bcd8033e.
> PCIe hung issue was observed on multiple platforms. The issue was
> reproduced
> when DUT was configured as AP and associated with 50+ STAs.
>
> With PCIe protocol analyzer, we can see DMA Read crossing 4KB boundary when
> issue happened. It broke PCIe spec and caused PCIe stuck.
>
> Tested:  IPQ8064 + QCA9984 with firmware 10.4-3.10-00047
>          QCS404 + QCA9984 with firmware 10.4-3.9.0.2--00044
>          Synaptics AS370 + QCA9888  with firmware 10.4-3.9.0.2--00040
>
> Signed-off-by: Zhi Chen <zhichen@codeaurora.org>
> ---
>  drivers/net/wireless/ath/ath10k/hw.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/hw.h
> b/drivers/net/wireless/ath/ath10k/hw.h
> index 35a3623..17f620b 100644
> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -753,7 +753,7 @@ ath10k_is_rssi_enable(struct ath10k_hw_params *hw,
>  #define TARGET_10X_MAX_FRAG_ENTRIES		0
>
>  /* 10.2 parameters */
> -#define TARGET_10_2_DMA_BURST_SIZE		0
> +#define TARGET_10_2_DMA_BURST_SIZE		1
>
>  /* Target specific defines for WMI-TLV firmware */
>  #define TARGET_TLV_NUM_VDEVS			4
> @@ -813,7 +813,7 @@ ath10k_is_rssi_enable(struct ath10k_hw_params *hw,
>
>  #define TARGET_10_4_TX_DBG_LOG_SIZE		1024
>  #define TARGET_10_4_NUM_WDS_ENTRIES		32
> -#define TARGET_10_4_DMA_BURST_SIZE		0
> +#define TARGET_10_4_DMA_BURST_SIZE		1
>  #define TARGET_10_4_MAC_AGGR_DELIM		0
>  #define TARGET_10_4_RX_SKIP_DEFRAG_TIMEOUT_DUP_DETECTION_CHECK 1
>  #define TARGET_10_4_VOW_CONFIG			0
> --
> 2.7.4
>
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
>

What about main and 10x firmware branch?
Peter Oh Oct. 22, 2019, 5:16 p.m. UTC | #2
On 10/22/19 1:57 AM, Zhi Chen wrote:
> This reverts commit 76d164f582150fd0259ec0fcbc485470bcd8033e.
> PCIe hung issue was observed on multiple platforms. The issue was reproduced
> when DUT was configured as AP and associated with 50+ STAs.
>
> With PCIe protocol analyzer, we can see DMA Read crossing 4KB boundary when
> issue happened. It broke PCIe spec and caused PCIe stuck.

How can you say value 0 (I believe it's 64 bytes) DMA burst size causes 
the symptom and 1 fixes it?

Peter
Adrian Chadd Oct. 22, 2019, 6:24 p.m. UTC | #3
On Tue, 22 Oct 2019 at 10:17, Peter Oh <peter.oh@eero.com> wrote:
>
>
> On 10/22/19 1:57 AM, Zhi Chen wrote:
> > This reverts commit 76d164f582150fd0259ec0fcbc485470bcd8033e.
> > PCIe hung issue was observed on multiple platforms. The issue was reproduced
> > when DUT was configured as AP and associated with 50+ STAs.
> >
> > With PCIe protocol analyzer, we can see DMA Read crossing 4KB boundary when
> > issue happened. It broke PCIe spec and caused PCIe stuck.
>
> How can you say value 0 (I believe it's 64 bytes) DMA burst size causes
> the symptom and 1 fixes it?


+1 to this question.

Also, shouldn't the DMA engine be doing what the firmware says? Is the
firmware/copy engine actually somehow bursting / prefetching across a
4K page boundary?

Surely this is something that can be fixed in software/firmware by
correctly configuring up buffer size/offsets?



-adrian
Zhi Chen Oct. 30, 2019, 2:44 a.m. UTC | #4
On 2019-10-22 18:07, Tom Psyborg wrote:

> What about main and 10x firmware branch?

There is no code changes in firmware. It's a configuration change of 
host memory access.

Zhi
Peter Oh Oct. 30, 2019, 6:04 a.m. UTC | #5
On 10/22/19 11:24 AM, Adrian Chadd wrote:
> On Tue, 22 Oct 2019 at 10:17, Peter Oh <peter.oh@eero.com> wrote:
>>
>> On 10/22/19 1:57 AM, Zhi Chen wrote:
>>> This reverts commit 76d164f582150fd0259ec0fcbc485470bcd8033e.
>>> PCIe hung issue was observed on multiple platforms. The issue was reproduced
>>> when DUT was configured as AP and associated with 50+ STAs.
>>>
>>> With PCIe protocol analyzer, we can see DMA Read crossing 4KB boundary when
>>> issue happened. It broke PCIe spec and caused PCIe stuck.
Can you elaborate how 64 bytes DMA size breaks PCIe spec?
>> How can you say value 0 (I believe it's 64 bytes) DMA burst size causes
>> the symptom and 1 fixes it?
>
> +1 to this question.
>
> Also, shouldn't the DMA engine be doing what the firmware says? Is the
> firmware/copy engine actually somehow bursting / prefetching across a
> 4K page boundary?
>
> Surely this is something that can be fixed in software/firmware by
> correctly configuring up buffer size/offsets?
>
>
>
> -adrian
Zhi Chen Oct. 30, 2019, 6:16 a.m. UTC | #6
On 2019-10-23 01:16, Peter Oh wrote:
> 
> How can you say value 0 (I believe it's 64 bytes) DMA burst size
> causes the symptom and 1 fixes it?
> 
> Peter

Confirmed from HW team that the configuration controls AXI burst size of 
the RD/WR access to the HOST MEM.
0-	No split , RAW read/write transfer size from MAC is put out on bus as 
burst length.
1-	Split at 256 byte boundary
2,3- Reserved

That's why we see issue with value 0.

Zhi
Zhi Chen Oct. 30, 2019, 6:28 a.m. UTC | #7
On 2019-10-23 02:24, Adrian Chadd wrote:
> On Tue, 22 Oct 2019 at 10:17, Peter Oh <peter.oh@eero.com> wrote:
>> 
>> 
>> On 10/22/19 1:57 AM, Zhi Chen wrote:
>> > This reverts commit 76d164f582150fd0259ec0fcbc485470bcd8033e.
>> > PCIe hung issue was observed on multiple platforms. The issue was reproduced
>> > when DUT was configured as AP and associated with 50+ STAs.
>> >
>> > With PCIe protocol analyzer, we can see DMA Read crossing 4KB boundary when
>> > issue happened. It broke PCIe spec and caused PCIe stuck.
>> 
>> How can you say value 0 (I believe it's 64 bytes) DMA burst size 
>> causes
>> the symptom and 1 fixes it?
> 
> 
> +1 to this question.
> 
> Also, shouldn't the DMA engine be doing what the firmware says? Is the
> firmware/copy engine actually somehow bursting / prefetching across a
> 4K page boundary?
> 
> Surely this is something that can be fixed in software/firmware by
> correctly configuring up buffer size/offsets?
> 
> 
> 
> -Adrian

DMA engine is working as expected as the configuration. It's copy engine 
which actually
splits the RD/WR requests and accesses host memory.
And yes it's platform related configuration. We have never hit this 
issue on x86 platform.


Zhi
Peter Oh Oct. 30, 2019, 11:01 p.m. UTC | #8
On 10/29/19 11:16 PM, zhichen@codeaurora.org wrote:
> On 2019-10-23 01:16, Peter Oh wrote:
>>
>> How can you say value 0 (I believe it's 64 bytes) DMA burst size
>> causes the symptom and 1 fixes it?
>>
>> Peter
>
> Confirmed from HW team that the configuration controls AXI burst size 
> of the RD/WR access to the HOST MEM.
> 0-    No split , RAW read/write transfer size from MAC is put out on 
> bus as burst length.
> 1-    Split at 256 byte boundary
> 2,3- Reserved
>
I would ACK to this change. Although QCA4019 host mem max AXI read/write 
registers have different descriptions from above, they say 1 (256-byte) 
is default and the values, 0, 2, and 3 are reserved. So changing it back 
to 1 is reasonable to me.


Thanks,

Peter
Tom Psyborg Nov. 3, 2019, 3:36 p.m. UTC | #9
On 30/10/2019, zhichen@codeaurora.org <zhichen@codeaurora.org> wrote:
> On 2019-10-23 01:16, Peter Oh wrote:
>>
>> How can you say value 0 (I believe it's 64 bytes) DMA burst size
>> causes the symptom and 1 fixes it?
>>
>> Peter
>
> Confirmed from HW team that the configuration controls AXI burst size of
> the RD/WR access to the HOST MEM.
> 0-	No split , RAW read/write transfer size from MAC is put out on bus as
> burst length.
> 1-	Split at 256 byte boundary
> 2,3- Reserved
>
> That's why we see issue with value 0.
>
> Zhi
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
>

is this true for both wave1 and wave2 ? at least per this commit
message: ath10k: Fix DMA burst size
it's suppose to be:

0 - 128B max (not sure if this means 128B static, or any size between 0 and 128)
1 - 256B
Peter Oh Nov. 4, 2019, 6:41 p.m. UTC | #10
On 11/3/19 7:36 AM, Tom Psyborg wrote:
> On 30/10/2019, zhichen@codeaurora.org <zhichen@codeaurora.org> wrote:
>> On 2019-10-23 01:16, Peter Oh wrote:
>>> How can you say value 0 (I believe it's 64 bytes) DMA burst size
>>> causes the symptom and 1 fixes it?
>>>
>>> Peter
>> Confirmed from HW team that the configuration controls AXI burst size of
>> the RD/WR access to the HOST MEM.
>> 0-	No split , RAW read/write transfer size from MAC is put out on bus as
>> burst length.
>> 1-	Split at 256 byte boundary
>> 2,3- Reserved
>>
>> That's why we see issue with value 0.
>>
>> Zhi
>>
>> _______________________________________________
>> ath10k mailing list
>> ath10k@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/ath10k
>>
> is this true for both wave1 and wave2 ? at least per this commit
> message: ath10k: Fix DMA burst size
> it's suppose to be:
>
> 0 - 128B max (not sure if this means 128B static, or any size between 0 and 128)
> 1 - 256B
I didn't check other chipsets, but QCA4019 registers say AXI read/write 
burst size is 64 bytes for 0.
Tom Psyborg Nov. 6, 2019, 10:01 a.m. UTC | #11
On 30/10/2019, zhichen@codeaurora.org <zhichen@codeaurora.org> wrote:
> On 2019-10-22 18:07, Tom Psyborg wrote:
>
>> What about main and 10x firmware branch?
>
> There is no code changes in firmware. It's a configuration change of
> host memory access.
>
> Zhi
>

Please change it for main and 10x branch too. Dumping hw1.0 regs with
QCA driver shows DMA burst value set to 1
Zhi Chen Nov. 7, 2019, 5:35 a.m. UTC | #12
On 2019-11-06 18:01, Tom Psyborg wrote:
> On 30/10/2019, zhichen@codeaurora.org <zhichen@codeaurora.org> wrote:
>> On 2019-10-22 18:07, Tom Psyborg wrote:
>> 
>>> What about main and 10x firmware branch?
>> 
>> There is no code changes in firmware. It's a configuration change of
>> host memory access.
>> 
>> Zhi
>> 
> 
> Please change it for main and 10x branch too. Dumping hw1.0 regs with
> QCA driver shows DMA burst value set to 1

Looks some old chips have different definitions of DMA burst size. We 
are discussing internally to change it for 10.4 only because we only 
tested 10.4.  On the other hand we are waiting for HW team to explain 
the differences.

Zhi
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 35a3623..17f620b 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -753,7 +753,7 @@  ath10k_is_rssi_enable(struct ath10k_hw_params *hw,
 #define TARGET_10X_MAX_FRAG_ENTRIES		0
 
 /* 10.2 parameters */
-#define TARGET_10_2_DMA_BURST_SIZE		0
+#define TARGET_10_2_DMA_BURST_SIZE		1
 
 /* Target specific defines for WMI-TLV firmware */
 #define TARGET_TLV_NUM_VDEVS			4
@@ -813,7 +813,7 @@  ath10k_is_rssi_enable(struct ath10k_hw_params *hw,
 
 #define TARGET_10_4_TX_DBG_LOG_SIZE		1024
 #define TARGET_10_4_NUM_WDS_ENTRIES		32
-#define TARGET_10_4_DMA_BURST_SIZE		0
+#define TARGET_10_4_DMA_BURST_SIZE		1
 #define TARGET_10_4_MAC_AGGR_DELIM		0
 #define TARGET_10_4_RX_SKIP_DEFRAG_TIMEOUT_DUP_DETECTION_CHECK 1
 #define TARGET_10_4_VOW_CONFIG			0