| Message ID | 20191219131539.1003793-2-bryan.odonoghue@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ath10k: pci: Two PCI related fixups | expand |
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > ath10k_pci_dump_memory_reg() will try to access memory of type > ATH10K_MEM_REGION_TYPE_IOREG however, if a hardware restart is in progress > this can crash a system. > > Individual ioread32() time has been observed to jump from 15-20 ticks to > > 80k ticks followed by a secure-watchdog bite and a system reset. > > Work around this corner case by only issuing the read transaction when the > driver state is ATH10K_STATE_ON. > > Fixes: 219cc084c6706 ("ath10k: add memory dump support QCA9984") > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> What ath10k hardware and firmware did you test this on? I can add that to the commit log.
On 19/12/2019 13:53, Kalle Valo wrote: > Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > >> ath10k_pci_dump_memory_reg() will try to access memory of type >> ATH10K_MEM_REGION_TYPE_IOREG however, if a hardware restart is in progress >> this can crash a system. >> >> Individual ioread32() time has been observed to jump from 15-20 ticks to > >> 80k ticks followed by a secure-watchdog bite and a system reset. >> >> Work around this corner case by only issuing the read transaction when the >> driver state is ATH10K_STATE_ON. >> >> Fixes: 219cc084c6706 ("ath10k: add memory dump support QCA9984") >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > What ath10k hardware and firmware did you test this on? I can add that > to the commit log. > HW = QCA9988 FW = ?? Not quite sure how to find the firmware version TBH
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > On 19/12/2019 13:53, Kalle Valo wrote: >> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: >> >>> ath10k_pci_dump_memory_reg() will try to access memory of type >>> ATH10K_MEM_REGION_TYPE_IOREG however, if a hardware restart is in progress >>> this can crash a system. >>> >>> Individual ioread32() time has been observed to jump from 15-20 ticks to > >>> 80k ticks followed by a secure-watchdog bite and a system reset. >>> >>> Work around this corner case by only issuing the read transaction when the >>> driver state is ATH10K_STATE_ON. >>> >>> Fixes: 219cc084c6706 ("ath10k: add memory dump support QCA9984") >>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> >> What ath10k hardware and firmware did you test this on? I can add that >> to the commit log. >> > > HW = QCA9988 > FW = ?? > > Not quite sure how to find the firmware version TBH 'dmesg | grep ath10k' should show it.
On 19/12/2019 14:21, Kalle Valo wrote:
> 'dmesg | grep ath10k' should show it.
[ 6.579772] ath10k_pci 0000:01:00.0: firmware ver 10.4-3.9.0.2-00044
api 5 features
no-p2p,mfp,peer-flow-ctrl,btcoex-param,allows-mesh-bcast,no-ps crc32
c3e1b393
Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote: > ath10k_pci_dump_memory_reg() will try to access memory of type > ATH10K_MEM_REGION_TYPE_IOREG however, if a hardware restart is in progress > this can crash a system. > > Individual ioread32() time has been observed to jump from 15-20 ticks to > > 80k ticks followed by a secure-watchdog bite and a system reset. > > Work around this corner case by only issuing the read transaction when the > driver state is ATH10K_STATE_ON. > > Tested-on: QCA9988 PCI 10.4-3.9.0.2-00044 > > Fixes: 219cc084c6706 ("ath10k: add memory dump support QCA9984") > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> 2 patches applied to ath-next branch of ath.git, thanks. d239380196c4 ath10k: pci: Only dump ATH10K_MEM_REGION_TYPE_IOREG when safe 63ec5cbc31f7 ath10k: pci: Fix comment on ath10k_pci_dump_memory_sram
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c index bb44f5a0941b..4822a65f6f3c 100644 --- a/drivers/net/wireless/ath/ath10k/pci.c +++ b/drivers/net/wireless/ath/ath10k/pci.c @@ -1604,11 +1604,22 @@ static int ath10k_pci_dump_memory_reg(struct ath10k *ar, { struct ath10k_pci *ar_pci = ath10k_pci_priv(ar); u32 i; + int ret; + + mutex_lock(&ar->conf_mutex); + if (ar->state != ATH10K_STATE_ON) { + ath10k_warn(ar, "Skipping pci_dump_memory_reg invalid state\n"); + ret = -EIO; + goto done; + } for (i = 0; i < region->len; i += 4) *(u32 *)(buf + i) = ioread32(ar_pci->mem + region->start + i); - return region->len; + ret = region->len; +done: + mutex_unlock(&ar->conf_mutex); + return ret; } /* if an error happened returns < 0, otherwise the length */ @@ -1704,7 +1715,11 @@ static void ath10k_pci_dump_memory(struct ath10k *ar, count = ath10k_pci_dump_memory_sram(ar, current_region, buf); break; case ATH10K_MEM_REGION_TYPE_IOREG: - count = ath10k_pci_dump_memory_reg(ar, current_region, buf); + ret = ath10k_pci_dump_memory_reg(ar, current_region, buf); + if (ret < 0) + break; + + count = ret; break; default: ret = ath10k_pci_dump_memory_generic(ar, current_region, buf);
ath10k_pci_dump_memory_reg() will try to access memory of type ATH10K_MEM_REGION_TYPE_IOREG however, if a hardware restart is in progress this can crash a system. Individual ioread32() time has been observed to jump from 15-20 ticks to > 80k ticks followed by a secure-watchdog bite and a system reset. Work around this corner case by only issuing the read transaction when the driver state is ATH10K_STATE_ON. Fixes: 219cc084c6706 ("ath10k: add memory dump support QCA9984") Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/net/wireless/ath/ath10k/pci.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)