All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets
@ 2021-09-13 18:02 ` Jouni Malinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

For fragmented packets, ath11k reassembles each fragment as a normal
packet and then reinjects it into HW ring. In this case, the DMA
direction should be DMA_TO_DEVICE, not DMA_FROM_DEVICE, otherwise
invalid payload will be reinjected to HW and then delivered to host.
What is more, since arbitrary memory could be allocated to the frame, we
don't know what kind of data is contained in the buffer reinjected.
Thus, as a bad result, private info may be leaked.

Note that this issue is only found on Intel platform.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/dp_rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 90da56316e7e..0c27eead3e02 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -3434,7 +3434,7 @@ static int ath11k_dp_rx_h_defrag_reo_reinject(struct ath11k *ar, struct dp_rx_ti
 
 	paddr = dma_map_single(ab->dev, defrag_skb->data,
 			       defrag_skb->len + skb_tailroom(defrag_skb),
-			       DMA_FROM_DEVICE);
+			       DMA_TO_DEVICE);
 	if (dma_mapping_error(ab->dev, paddr))
 		return -ENOMEM;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets
@ 2021-09-13 18:02 ` Jouni Malinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

For fragmented packets, ath11k reassembles each fragment as a normal
packet and then reinjects it into HW ring. In this case, the DMA
direction should be DMA_TO_DEVICE, not DMA_FROM_DEVICE, otherwise
invalid payload will be reinjected to HW and then delivered to host.
What is more, since arbitrary memory could be allocated to the frame, we
don't know what kind of data is contained in the buffer reinjected.
Thus, as a bad result, private info may be leaked.

Note that this issue is only found on Intel platform.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/dp_rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 90da56316e7e..0c27eead3e02 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -3434,7 +3434,7 @@ static int ath11k_dp_rx_h_defrag_reo_reinject(struct ath11k *ar, struct dp_rx_ti
 
 	paddr = dma_map_single(ab->dev, defrag_skb->data,
 			       defrag_skb->len + skb_tailroom(defrag_skb),
-			       DMA_FROM_DEVICE);
+			       DMA_TO_DEVICE);
 	if (dma_mapping_error(ab->dev, paddr))
 		return -ENOMEM;
 
-- 
2.25.1


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/5] ath11k: Drop MSDU with length error in DP rx path
  2021-09-13 18:02 ` Jouni Malinen
@ 2021-09-13 18:02   ` Jouni Malinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

There are MSDUs whose length are invalid. For example,
attackers may inject on purpose truncated A-MSDUs with
invalid MSDU length.

Such MSDUs are marked with an err bit set in rx attention
tlvs, so we can check and drop them.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/dp_rx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 0c27eead3e02..c50f70913583 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -142,6 +142,18 @@ static u32 ath11k_dp_rx_h_attn_mpdu_err(struct rx_attention *attn)
 	return errmap;
 }
 
+static bool ath11k_dp_rx_h_attn_msdu_len_err(struct ath11k_base *ab,
+					     struct hal_rx_desc *desc)
+{
+	struct rx_attention *rx_attention;
+	u32 errmap;
+
+	rx_attention = ath11k_dp_rx_get_attention(ab, desc);
+	errmap = ath11k_dp_rx_h_attn_mpdu_err(rx_attention);
+
+	return errmap & DP_RX_MPDU_ERR_MSDU_LEN;
+}
+
 static u16 ath11k_dp_rx_h_msdu_start_msdu_len(struct ath11k_base *ab,
 					      struct hal_rx_desc *desc)
 {
@@ -2525,6 +2537,12 @@ static int ath11k_dp_rx_process_msdu(struct ath11k *ar,
 	}
 
 	rx_desc = (struct hal_rx_desc *)msdu->data;
+	if (ath11k_dp_rx_h_attn_msdu_len_err(ab, rx_desc)) {
+		ath11k_warn(ar->ab, "msdu len not valid\n");
+		ret = -EIO;
+		goto free_out;
+	}
+
 	lrx_desc = (struct hal_rx_desc *)last_buf->data;
 	rx_attention = ath11k_dp_rx_get_attention(ab, lrx_desc);
 	if (!ath11k_dp_rx_h_attn_msdu_done(rx_attention)) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/5] ath11k: Drop MSDU with length error in DP rx path
@ 2021-09-13 18:02   ` Jouni Malinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

There are MSDUs whose length are invalid. For example,
attackers may inject on purpose truncated A-MSDUs with
invalid MSDU length.

Such MSDUs are marked with an err bit set in rx attention
tlvs, so we can check and drop them.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/dp_rx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 0c27eead3e02..c50f70913583 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -142,6 +142,18 @@ static u32 ath11k_dp_rx_h_attn_mpdu_err(struct rx_attention *attn)
 	return errmap;
 }
 
+static bool ath11k_dp_rx_h_attn_msdu_len_err(struct ath11k_base *ab,
+					     struct hal_rx_desc *desc)
+{
+	struct rx_attention *rx_attention;
+	u32 errmap;
+
+	rx_attention = ath11k_dp_rx_get_attention(ab, desc);
+	errmap = ath11k_dp_rx_h_attn_mpdu_err(rx_attention);
+
+	return errmap & DP_RX_MPDU_ERR_MSDU_LEN;
+}
+
 static u16 ath11k_dp_rx_h_msdu_start_msdu_len(struct ath11k_base *ab,
 					      struct hal_rx_desc *desc)
 {
@@ -2525,6 +2537,12 @@ static int ath11k_dp_rx_process_msdu(struct ath11k *ar,
 	}
 
 	rx_desc = (struct hal_rx_desc *)msdu->data;
+	if (ath11k_dp_rx_h_attn_msdu_len_err(ab, rx_desc)) {
+		ath11k_warn(ar->ab, "msdu len not valid\n");
+		ret = -EIO;
+		goto free_out;
+	}
+
 	lrx_desc = (struct hal_rx_desc *)last_buf->data;
 	rx_attention = ath11k_dp_rx_get_attention(ab, lrx_desc);
 	if (!ath11k_dp_rx_h_attn_msdu_done(rx_attention)) {
-- 
2.25.1


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/5] ath11k: Fix inaccessible debug registers
  2021-09-13 18:02 ` Jouni Malinen
@ 2021-09-13 18:02   ` Jouni Malinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

Current code clears debug registers after SOC global reset performed
in ath11k_pci_sw_reset. However at that time those registers are
not accessible due to reset, thus they are actually not cleared at all.
For WCN6855, it may cause target fail to initialize. This issue can be
fixed by moving clear action ahead.

In addition, on some specific platforms, need to add delay to wait
those registers to become accessible.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index 5abb38cc3b55..7b3bce0ba76e 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -430,6 +430,8 @@ static void ath11k_pci_force_wake(struct ath11k_base *ab)
 
 static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on)
 {
+	mdelay(100);
+
 	if (power_on) {
 		ath11k_pci_enable_ltssm(ab);
 		ath11k_pci_clear_all_intrs(ab);
@@ -439,9 +441,9 @@ static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on)
 	}
 
 	ath11k_mhi_clear_vector(ab);
+	ath11k_pci_clear_dbg_registers(ab);
 	ath11k_pci_soc_global_reset(ab);
 	ath11k_mhi_set_mhictrl_reset(ab);
-	ath11k_pci_clear_dbg_registers(ab);
 }
 
 int ath11k_pci_get_msi_irq(struct device *dev, unsigned int vector)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/5] ath11k: Fix inaccessible debug registers
@ 2021-09-13 18:02   ` Jouni Malinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

Current code clears debug registers after SOC global reset performed
in ath11k_pci_sw_reset. However at that time those registers are
not accessible due to reset, thus they are actually not cleared at all.
For WCN6855, it may cause target fail to initialize. This issue can be
fixed by moving clear action ahead.

In addition, on some specific platforms, need to add delay to wait
those registers to become accessible.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index 5abb38cc3b55..7b3bce0ba76e 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -430,6 +430,8 @@ static void ath11k_pci_force_wake(struct ath11k_base *ab)
 
 static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on)
 {
+	mdelay(100);
+
 	if (power_on) {
 		ath11k_pci_enable_ltssm(ab);
 		ath11k_pci_clear_all_intrs(ab);
@@ -439,9 +441,9 @@ static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on)
 	}
 
 	ath11k_mhi_clear_vector(ab);
+	ath11k_pci_clear_dbg_registers(ab);
 	ath11k_pci_soc_global_reset(ab);
 	ath11k_mhi_set_mhictrl_reset(ab);
-	ath11k_pci_clear_dbg_registers(ab);
 }
 
 int ath11k_pci_get_msi_irq(struct device *dev, unsigned int vector)
-- 
2.25.1


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/5] ath11k: Fix memory leak in ath11k_qmi_driver_event_work
  2021-09-13 18:02 ` Jouni Malinen
@ 2021-09-13 18:02   ` Jouni Malinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

The buffer pointed to by event is not freed in case
ATH11K_FLAG_UNREGISTERING bit is set, resulting in
memory leak, so fix it.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/qmi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 7e82d03b0d87..0175e35849bf 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -2753,8 +2753,10 @@ static void ath11k_qmi_driver_event_work(struct work_struct *work)
 		list_del(&event->list);
 		spin_unlock(&qmi->event_lock);
 
-		if (test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags))
+		if (test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags)) {
+			kfree(event);
 			return;
+		}
 
 		switch (event->type) {
 		case ATH11K_QMI_EVENT_SERVER_ARRIVE:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/5] ath11k: Fix memory leak in ath11k_qmi_driver_event_work
@ 2021-09-13 18:02   ` Jouni Malinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

The buffer pointed to by event is not freed in case
ATH11K_FLAG_UNREGISTERING bit is set, resulting in
memory leak, so fix it.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/qmi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 7e82d03b0d87..0175e35849bf 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -2753,8 +2753,10 @@ static void ath11k_qmi_driver_event_work(struct work_struct *work)
 		list_del(&event->list);
 		spin_unlock(&qmi->event_lock);
 
-		if (test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags))
+		if (test_bit(ATH11K_FLAG_UNREGISTERING, &ab->dev_flags)) {
+			kfree(event);
 			return;
+		}
 
 		switch (event->type) {
 		case ATH11K_QMI_EVENT_SERVER_ARRIVE:
-- 
2.25.1


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/5] ath11k: Handle MSI enablement during rmmod and SSR
  2021-09-13 18:02 ` Jouni Malinen
@ 2021-09-13 18:02   ` Jouni Malinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

When doing "rmmod ath11k_pci", ath11k performs global SOC reset
and MHI reset, where 0 address access is captured by IOMMU. See
log below:

...
[  133.953860] ath11k_pci 0000:02:00.0: setting mhi state: DEINIT(1)
[  133.959714] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
[  133.973854] ath11k_pci 0000:02:00.0: MHISTATUS 0xff04
[  133.974095] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
...

This issue is also observed in SSR process, cause a similar
sequence as above is performed.

Such an invalid access occurs because, during rmmod or SSR, MSI
address is cleared but HW MSI functionality not disabled, thus HW
target is able to raise an MSI transaction with 0 as MSI address.

So it can be fixed by simply disabling MSI before reset. For SSR,
since MSI functionality is still needed after target is brought
back, we need to reenable it.

Also change naming of some interfaces related.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/pci.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index 7b3bce0ba76e..1094b53465bc 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -855,7 +855,18 @@ static void ath11k_pci_ce_irqs_enable(struct ath11k_base *ab)
 	}
 }
 
-static int ath11k_pci_enable_msi(struct ath11k_pci *ab_pci)
+static void ath11k_pci_enable_msi(struct pci_dev *dev, bool enable)
+{
+	u16 control;
+
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
+	control &= ~PCI_MSI_FLAGS_ENABLE;
+	if (enable)
+		control |= PCI_MSI_FLAGS_ENABLE;
+	pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
+}
+
+static int ath11k_pci_alloc_msi(struct ath11k_pci *ab_pci)
 {
 	struct ath11k_base *ab = ab_pci->ab;
 	const struct ath11k_msi_config *msi_config = ab_pci->msi_config;
@@ -876,6 +887,7 @@ static int ath11k_pci_enable_msi(struct ath11k_pci *ab_pci)
 		else
 			return num_vectors;
 	}
+	ath11k_pci_enable_msi(ab_pci->pdev, false);
 
 	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
 	if (!msi_desc) {
@@ -898,7 +910,7 @@ static int ath11k_pci_enable_msi(struct ath11k_pci *ab_pci)
 	return ret;
 }
 
-static void ath11k_pci_disable_msi(struct ath11k_pci *ab_pci)
+static void ath11k_pci_free_msi(struct ath11k_pci *ab_pci)
 {
 	pci_free_irq_vectors(ab_pci->pdev);
 }
@@ -1019,6 +1031,8 @@ static int ath11k_pci_power_up(struct ath11k_base *ab)
 	 */
 	ath11k_pci_aspm_disable(ab_pci);
 
+	ath11k_pci_enable_msi(ab_pci->pdev, true);
+
 	ret = ath11k_mhi_start(ab_pci);
 	if (ret) {
 		ath11k_err(ab, "failed to start mhi: %d\n", ret);
@@ -1039,6 +1053,9 @@ static void ath11k_pci_power_down(struct ath11k_base *ab)
 	ath11k_pci_aspm_restore(ab_pci);
 
 	ath11k_pci_force_wake(ab_pci->ab);
+
+	ath11k_pci_enable_msi(ab_pci->pdev, false);
+
 	ath11k_mhi_stop(ab_pci);
 	clear_bit(ATH11K_PCI_FLAG_INIT_DONE, &ab_pci->flags);
 	ath11k_pci_sw_reset(ab_pci->ab, false);
@@ -1263,7 +1280,7 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
 		goto err_pci_free_region;
 	}
 
-	ret = ath11k_pci_enable_msi(ab_pci);
+	ret = ath11k_pci_alloc_msi(ab_pci);
 	if (ret) {
 		ath11k_err(ab, "failed to enable msi: %d\n", ret);
 		goto err_pci_free_region;
@@ -1317,7 +1334,7 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
 	ath11k_mhi_unregister(ab_pci);
 
 err_pci_disable_msi:
-	ath11k_pci_disable_msi(ab_pci);
+	ath11k_pci_free_msi(ab_pci);
 
 err_pci_free_region:
 	ath11k_pci_free_region(ab_pci);
@@ -1348,7 +1365,7 @@ static void ath11k_pci_remove(struct pci_dev *pdev)
 	ath11k_mhi_unregister(ab_pci);
 
 	ath11k_pci_free_irq(ab);
-	ath11k_pci_disable_msi(ab_pci);
+	ath11k_pci_free_msi(ab_pci);
 	ath11k_pci_free_region(ab_pci);
 
 	ath11k_hal_srng_deinit(ab);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/5] ath11k: Handle MSI enablement during rmmod and SSR
@ 2021-09-13 18:02   ` Jouni Malinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jouni Malinen @ 2021-09-13 18:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

From: Baochen Qiang <bqiang@codeaurora.org>

When doing "rmmod ath11k_pci", ath11k performs global SOC reset
and MHI reset, where 0 address access is captured by IOMMU. See
log below:

...
[  133.953860] ath11k_pci 0000:02:00.0: setting mhi state: DEINIT(1)
[  133.959714] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
[  133.973854] ath11k_pci 0000:02:00.0: MHISTATUS 0xff04
[  133.974095] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
...

This issue is also observed in SSR process, cause a similar
sequence as above is performed.

Such an invalid access occurs because, during rmmod or SSR, MSI
address is cleared but HW MSI functionality not disabled, thus HW
target is able to raise an MSI transaction with 0 as MSI address.

So it can be fixed by simply disabling MSI before reset. For SSR,
since MSI functionality is still needed after target is brought
back, we need to reenable it.

Also change naming of some interfaces related.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/pci.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index 7b3bce0ba76e..1094b53465bc 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -855,7 +855,18 @@ static void ath11k_pci_ce_irqs_enable(struct ath11k_base *ab)
 	}
 }
 
-static int ath11k_pci_enable_msi(struct ath11k_pci *ab_pci)
+static void ath11k_pci_enable_msi(struct pci_dev *dev, bool enable)
+{
+	u16 control;
+
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
+	control &= ~PCI_MSI_FLAGS_ENABLE;
+	if (enable)
+		control |= PCI_MSI_FLAGS_ENABLE;
+	pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
+}
+
+static int ath11k_pci_alloc_msi(struct ath11k_pci *ab_pci)
 {
 	struct ath11k_base *ab = ab_pci->ab;
 	const struct ath11k_msi_config *msi_config = ab_pci->msi_config;
@@ -876,6 +887,7 @@ static int ath11k_pci_enable_msi(struct ath11k_pci *ab_pci)
 		else
 			return num_vectors;
 	}
+	ath11k_pci_enable_msi(ab_pci->pdev, false);
 
 	msi_desc = irq_get_msi_desc(ab_pci->pdev->irq);
 	if (!msi_desc) {
@@ -898,7 +910,7 @@ static int ath11k_pci_enable_msi(struct ath11k_pci *ab_pci)
 	return ret;
 }
 
-static void ath11k_pci_disable_msi(struct ath11k_pci *ab_pci)
+static void ath11k_pci_free_msi(struct ath11k_pci *ab_pci)
 {
 	pci_free_irq_vectors(ab_pci->pdev);
 }
@@ -1019,6 +1031,8 @@ static int ath11k_pci_power_up(struct ath11k_base *ab)
 	 */
 	ath11k_pci_aspm_disable(ab_pci);
 
+	ath11k_pci_enable_msi(ab_pci->pdev, true);
+
 	ret = ath11k_mhi_start(ab_pci);
 	if (ret) {
 		ath11k_err(ab, "failed to start mhi: %d\n", ret);
@@ -1039,6 +1053,9 @@ static void ath11k_pci_power_down(struct ath11k_base *ab)
 	ath11k_pci_aspm_restore(ab_pci);
 
 	ath11k_pci_force_wake(ab_pci->ab);
+
+	ath11k_pci_enable_msi(ab_pci->pdev, false);
+
 	ath11k_mhi_stop(ab_pci);
 	clear_bit(ATH11K_PCI_FLAG_INIT_DONE, &ab_pci->flags);
 	ath11k_pci_sw_reset(ab_pci->ab, false);
@@ -1263,7 +1280,7 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
 		goto err_pci_free_region;
 	}
 
-	ret = ath11k_pci_enable_msi(ab_pci);
+	ret = ath11k_pci_alloc_msi(ab_pci);
 	if (ret) {
 		ath11k_err(ab, "failed to enable msi: %d\n", ret);
 		goto err_pci_free_region;
@@ -1317,7 +1334,7 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
 	ath11k_mhi_unregister(ab_pci);
 
 err_pci_disable_msi:
-	ath11k_pci_disable_msi(ab_pci);
+	ath11k_pci_free_msi(ab_pci);
 
 err_pci_free_region:
 	ath11k_pci_free_region(ab_pci);
@@ -1348,7 +1365,7 @@ static void ath11k_pci_remove(struct pci_dev *pdev)
 	ath11k_mhi_unregister(ab_pci);
 
 	ath11k_pci_free_irq(ab);
-	ath11k_pci_disable_msi(ab_pci);
+	ath11k_pci_free_msi(ab_pci);
 	ath11k_pci_free_region(ab_pci);
 
 	ath11k_hal_srng_deinit(ab);
-- 
2.25.1


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets
  2021-09-13 18:02 ` Jouni Malinen
@ 2021-09-13 22:38   ` Peter Oh
  -1 siblings, 0 replies; 20+ messages in thread
From: Peter Oh @ 2021-09-13 22:38 UTC (permalink / raw)
  To: Jouni Malinen, Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang


> diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
> index 90da56316e7e..0c27eead3e02 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
> @@ -3434,7 +3434,7 @@ static int ath11k_dp_rx_h_defrag_reo_reinject(struct ath11k *ar, struct dp_rx_ti
>   
>   	paddr = dma_map_single(ab->dev, defrag_skb->data,
>   			       defrag_skb->len + skb_tailroom(defrag_skb),
> -			       DMA_FROM_DEVICE);
> +			       DMA_TO_DEVICE);
>   	if (dma_mapping_error(ab->dev, paddr))
>   		return -ENOMEM;
>   

Need to update this line too?

err_unmap_dma:
     dma_unmap_single(ab->dev, paddr, defrag_skb->len + 
skb_tailroom(defrag_skb),
              DMA_FROM_DEVICE);


Thanks,

Peter


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets
@ 2021-09-13 22:38   ` Peter Oh
  0 siblings, 0 replies; 20+ messages in thread
From: Peter Oh @ 2021-09-13 22:38 UTC (permalink / raw)
  To: Jouni Malinen, Kalle Valo; +Cc: ath11k, linux-wireless, Baochen Qiang


> diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
> index 90da56316e7e..0c27eead3e02 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
> @@ -3434,7 +3434,7 @@ static int ath11k_dp_rx_h_defrag_reo_reinject(struct ath11k *ar, struct dp_rx_ti
>   
>   	paddr = dma_map_single(ab->dev, defrag_skb->data,
>   			       defrag_skb->len + skb_tailroom(defrag_skb),
> -			       DMA_FROM_DEVICE);
> +			       DMA_TO_DEVICE);
>   	if (dma_mapping_error(ab->dev, paddr))
>   		return -ENOMEM;
>   

Need to update this line too?

err_unmap_dma:
     dma_unmap_single(ab->dev, paddr, defrag_skb->len + 
skb_tailroom(defrag_skb),
              DMA_FROM_DEVICE);


Thanks,

Peter


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets
  2021-09-13 18:02 ` Jouni Malinen
                   ` (5 preceding siblings ...)
  (?)
@ 2021-09-14 17:09 ` Kalle Valo
  -1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2021-09-14 17:09 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

Jouni Malinen <jouni@codeaurora.org> wrote:

> For fragmented packets, ath11k reassembles each fragment as a normal
> packet and then reinjects it into HW ring. In this case, the DMA
> direction should be DMA_TO_DEVICE, not DMA_FROM_DEVICE, otherwise
> invalid payload will be reinjected to HW and then delivered to host.
> What is more, since arbitrary memory could be allocated to the frame, we
> don't know what kind of data is contained in the buffer reinjected.
> Thus, as a bad result, private info may be leaked.
> 
> Note that this issue is only found on Intel platform.
> 
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Dropping due to the issue Peter found.

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210913180246.193388-1-jouni@codeaurora.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets
  2021-09-13 18:02 ` Jouni Malinen
                   ` (6 preceding siblings ...)
  (?)
@ 2021-09-14 17:09 ` Kalle Valo
  -1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2021-09-14 17:09 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

Jouni Malinen <jouni@codeaurora.org> wrote:

> For fragmented packets, ath11k reassembles each fragment as a normal
> packet and then reinjects it into HW ring. In this case, the DMA
> direction should be DMA_TO_DEVICE, not DMA_FROM_DEVICE, otherwise
> invalid payload will be reinjected to HW and then delivered to host.
> What is more, since arbitrary memory could be allocated to the frame, we
> don't know what kind of data is contained in the buffer reinjected.
> Thus, as a bad result, private info may be leaked.
> 
> Note that this issue is only found on Intel platform.
> 
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Dropping due to the issue Peter found.

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210913180246.193388-1-jouni@codeaurora.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] ath11k: Drop MSDU with length error in DP rx path
  2021-09-13 18:02   ` Jouni Malinen
  (?)
@ 2021-09-28 13:34   ` Kalle Valo
  -1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2021-09-28 13:34 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

Jouni Malinen <jouni@codeaurora.org> wrote:

> There are MSDUs whose length are invalid. For example,
> attackers may inject on purpose truncated A-MSDUs with
> invalid MSDU length.
> 
> Such MSDUs are marked with an err bit set in rx attention
> tlvs, so we can check and drop them.
> 
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
> 
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

3 patches applied to ath-next branch of ath.git, thanks.

cd18ed4cf805 ath11k: Drop MSDU with length error in DP rx path
8a0b899f169d ath11k: Fix inaccessible debug registers
72de799aa9e3 ath11k: Fix memory leak in ath11k_qmi_driver_event_work

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210913180246.193388-2-jouni@codeaurora.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] ath11k: Drop MSDU with length error in DP rx path
  2021-09-13 18:02   ` Jouni Malinen
  (?)
  (?)
@ 2021-09-28 13:34   ` Kalle Valo
  -1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2021-09-28 13:34 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

Jouni Malinen <jouni@codeaurora.org> wrote:

> There are MSDUs whose length are invalid. For example,
> attackers may inject on purpose truncated A-MSDUs with
> invalid MSDU length.
> 
> Such MSDUs are marked with an err bit set in rx attention
> tlvs, so we can check and drop them.
> 
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
> 
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

3 patches applied to ath-next branch of ath.git, thanks.

cd18ed4cf805 ath11k: Drop MSDU with length error in DP rx path
8a0b899f169d ath11k: Fix inaccessible debug registers
72de799aa9e3 ath11k: Fix memory leak in ath11k_qmi_driver_event_work

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210913180246.193388-2-jouni@codeaurora.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] ath11k: Handle MSI enablement during rmmod and SSR
  2021-09-13 18:02   ` Jouni Malinen
@ 2021-10-11  6:47     ` Kalle Valo
  -1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2021-10-11  6:47 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: ath11k, linux-wireless, Baochen Qiang

Jouni Malinen <jouni@codeaurora.org> writes:

> From: Baochen Qiang <bqiang@codeaurora.org>
>
> When doing "rmmod ath11k_pci", ath11k performs global SOC reset
> and MHI reset, where 0 address access is captured by IOMMU. See
> log below:
>
> ...
> [  133.953860] ath11k_pci 0000:02:00.0: setting mhi state: DEINIT(1)
> [  133.959714] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
> [  133.973854] ath11k_pci 0000:02:00.0: MHISTATUS 0xff04
> [  133.974095] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
> ...
>
> This issue is also observed in SSR process, cause a similar
> sequence as above is performed.
>
> Such an invalid access occurs because, during rmmod or SSR, MSI
> address is cleared but HW MSI functionality not disabled, thus HW
> target is able to raise an MSI transaction with 0 as MSI address.
>
> So it can be fixed by simply disabling MSI before reset. For SSR,
> since MSI functionality is still needed after target is brought
> back, we need to reenable it.
>
> Also change naming of some interfaces related.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> ---
>  drivers/net/wireless/ath/ath11k/pci.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
> index 7b3bce0ba76e..1094b53465bc 100644
> --- a/drivers/net/wireless/ath/ath11k/pci.c
> +++ b/drivers/net/wireless/ath/ath11k/pci.c
> @@ -855,7 +855,18 @@ static void ath11k_pci_ce_irqs_enable(struct ath11k_base *ab)
>  	}
>  }
>  
> -static int ath11k_pci_enable_msi(struct ath11k_pci *ab_pci)
> +static void ath11k_pci_enable_msi(struct pci_dev *dev, bool enable)
> +{
> +	u16 control;
> +
> +	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
> +	control &= ~PCI_MSI_FLAGS_ENABLE;
> +	if (enable)
> +		control |= PCI_MSI_FLAGS_ENABLE;
> +	pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
> +}

To make the function cleaner I renamed this to ath11k_pci_msi_config(),
added an else branch and changed it to take structh ath11k_pci. I also
added helpers ath11k_pci_msi_enable() and ath11k_pci_msi_disable().

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] ath11k: Handle MSI enablement during rmmod and SSR
@ 2021-10-11  6:47     ` Kalle Valo
  0 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2021-10-11  6:47 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: ath11k, linux-wireless, Baochen Qiang

Jouni Malinen <jouni@codeaurora.org> writes:

> From: Baochen Qiang <bqiang@codeaurora.org>
>
> When doing "rmmod ath11k_pci", ath11k performs global SOC reset
> and MHI reset, where 0 address access is captured by IOMMU. See
> log below:
>
> ...
> [  133.953860] ath11k_pci 0000:02:00.0: setting mhi state: DEINIT(1)
> [  133.959714] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
> [  133.973854] ath11k_pci 0000:02:00.0: MHISTATUS 0xff04
> [  133.974095] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
> ...
>
> This issue is also observed in SSR process, cause a similar
> sequence as above is performed.
>
> Such an invalid access occurs because, during rmmod or SSR, MSI
> address is cleared but HW MSI functionality not disabled, thus HW
> target is able to raise an MSI transaction with 0 as MSI address.
>
> So it can be fixed by simply disabling MSI before reset. For SSR,
> since MSI functionality is still needed after target is brought
> back, we need to reenable it.
>
> Also change naming of some interfaces related.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> ---
>  drivers/net/wireless/ath/ath11k/pci.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
> index 7b3bce0ba76e..1094b53465bc 100644
> --- a/drivers/net/wireless/ath/ath11k/pci.c
> +++ b/drivers/net/wireless/ath/ath11k/pci.c
> @@ -855,7 +855,18 @@ static void ath11k_pci_ce_irqs_enable(struct ath11k_base *ab)
>  	}
>  }
>  
> -static int ath11k_pci_enable_msi(struct ath11k_pci *ab_pci)
> +static void ath11k_pci_enable_msi(struct pci_dev *dev, bool enable)
> +{
> +	u16 control;
> +
> +	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
> +	control &= ~PCI_MSI_FLAGS_ENABLE;
> +	if (enable)
> +		control |= PCI_MSI_FLAGS_ENABLE;
> +	pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
> +}

To make the function cleaner I renamed this to ath11k_pci_msi_config(),
added an else branch and changed it to take structh ath11k_pci. I also
added helpers ath11k_pci_msi_enable() and ath11k_pci_msi_disable().

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] ath11k: Handle MSI enablement during rmmod and SSR
  2021-09-13 18:02   ` Jouni Malinen
@ 2021-10-11 15:11     ` Kalle Valo
  -1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2021-10-11 15:11 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

Jouni Malinen <jouni@codeaurora.org> wrote:

> When doing "rmmod ath11k_pci", ath11k performs global SOC reset
> and MHI reset, where 0 address access is captured by IOMMU. See
> log below:
> 
> ...
> [  133.953860] ath11k_pci 0000:02:00.0: setting mhi state: DEINIT(1)
> [  133.959714] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
> [  133.973854] ath11k_pci 0000:02:00.0: MHISTATUS 0xff04
> [  133.974095] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
> ...
> 
> This issue is also observed in SSR process, cause a similar
> sequence as above is performed.
> 
> Such an invalid access occurs because, during rmmod or SSR, MSI
> address is cleared but HW MSI functionality not disabled, thus HW
> target is able to raise an MSI transaction with 0 as MSI address.
> 
> So it can be fixed by simply disabling MSI before reset. For SSR,
> since MSI functionality is still needed after target is brought
> back, we need to reenable it.
> 
> Also change naming of some interfaces related.
> 
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
> 
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

96527d527b27 ath11k: Handle MSI enablement during rmmod and SSR

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210913180246.193388-5-jouni@codeaurora.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] ath11k: Handle MSI enablement during rmmod and SSR
@ 2021-10-11 15:11     ` Kalle Valo
  0 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2021-10-11 15:11 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: ath11k, linux-wireless, Baochen Qiang, Jouni Malinen

Jouni Malinen <jouni@codeaurora.org> wrote:

> When doing "rmmod ath11k_pci", ath11k performs global SOC reset
> and MHI reset, where 0 address access is captured by IOMMU. See
> log below:
> 
> ...
> [  133.953860] ath11k_pci 0000:02:00.0: setting mhi state: DEINIT(1)
> [  133.959714] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
> [  133.973854] ath11k_pci 0000:02:00.0: MHISTATUS 0xff04
> [  133.974095] ath11k_pci 0000:02:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000a address=0x0 flags=0x0020]
> ...
> 
> This issue is also observed in SSR process, cause a similar
> sequence as above is performed.
> 
> Such an invalid access occurs because, during rmmod or SSR, MSI
> address is cleared but HW MSI functionality not disabled, thus HW
> target is able to raise an MSI transaction with 0 as MSI address.
> 
> So it can be fixed by simply disabling MSI before reset. For SSR,
> since MSI functionality is still needed after target is brought
> back, we need to reenable it.
> 
> Also change naming of some interfaces related.
> 
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
> 
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

96527d527b27 ath11k: Handle MSI enablement during rmmod and SSR

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210913180246.193388-5-jouni@codeaurora.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-10-11 15:12 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 18:02 [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets Jouni Malinen
2021-09-13 18:02 ` Jouni Malinen
2021-09-13 18:02 ` [PATCH 2/5] ath11k: Drop MSDU with length error in DP rx path Jouni Malinen
2021-09-13 18:02   ` Jouni Malinen
2021-09-28 13:34   ` Kalle Valo
2021-09-28 13:34   ` Kalle Valo
2021-09-13 18:02 ` [PATCH 3/5] ath11k: Fix inaccessible debug registers Jouni Malinen
2021-09-13 18:02   ` Jouni Malinen
2021-09-13 18:02 ` [PATCH 4/5] ath11k: Fix memory leak in ath11k_qmi_driver_event_work Jouni Malinen
2021-09-13 18:02   ` Jouni Malinen
2021-09-13 18:02 ` [PATCH 5/5] ath11k: Handle MSI enablement during rmmod and SSR Jouni Malinen
2021-09-13 18:02   ` Jouni Malinen
2021-10-11  6:47   ` Kalle Valo
2021-10-11  6:47     ` Kalle Valo
2021-10-11 15:11   ` Kalle Valo
2021-10-11 15:11     ` Kalle Valo
2021-09-13 22:38 ` [PATCH 1/5] ath11k: Change DMA_FROM_DEVICE to DMA_TO_DEVICE when map reinjected packets Peter Oh
2021-09-13 22:38   ` Peter Oh
2021-09-14 17:09 ` Kalle Valo
2021-09-14 17:09 ` Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.