2023-12-12 03:19:28

by Zhenghao Gu

[permalink] [raw]
Subject: [PATCH v2] wifi: ath11k: fix IOMMU errors on buffer rings

virt_to_phys doesn't work on systems with IOMMU enabled,
which have non-identity physical-to-IOVA mappings.
It leads to IO_PAGE_FAULTs like this:
[IO_PAGE_FAULT domain=0x0023 address=0x1cce00000 flags=0x0020]
and no link can be established.

This patch changes that to dma_map_single(), which works correctly.

Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
Signed-off-by: Zhenghao Gu <[email protected]>
---
Changes in v2:
- ath11k_hal_srng_dst_peek_with_dma() is made static
---
drivers/net/wireless/ath/ath11k/dp.c | 19 ++++++++++++++++---
drivers/net/wireless/ath/ath11k/hal.c | 20 ++++++++++++++++++--
2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp.c b/drivers/net/wireless/ath/ath11k/dp.c
index d070bcb3f..c74230e4c 100644
--- a/drivers/net/wireless/ath/ath11k/dp.c
+++ b/drivers/net/wireless/ath/ath11k/dp.c
@@ -104,11 +104,14 @@ void ath11k_dp_srng_cleanup(struct ath11k_base *ab, struct dp_srng *ring)
if (!ring->vaddr_unaligned)
return;

- if (ring->cached)
+ if (ring->cached) {
+ dma_unmap_single(ab->dev, ring->paddr_unaligned, ring->size,
+ DMA_FROM_DEVICE);
kfree(ring->vaddr_unaligned);
- else
+ } else {
dma_free_coherent(ab->dev, ring->size, ring->vaddr_unaligned,
ring->paddr_unaligned);
+ }

ring->vaddr_unaligned = NULL;
}
@@ -249,7 +252,17 @@ int ath11k_dp_srng_setup(struct ath11k_base *ab, struct dp_srng *ring,

if (cached) {
ring->vaddr_unaligned = kzalloc(ring->size, GFP_KERNEL);
- ring->paddr_unaligned = virt_to_phys(ring->vaddr_unaligned);
+ if (!ring->vaddr_unaligned)
+ return -ENOMEM;
+
+ ring->paddr_unaligned =
+ dma_map_single(ab->dev, ring->vaddr_unaligned,
+ ring->size, DMA_FROM_DEVICE);
+ if (dma_mapping_error(ab->dev, ring->paddr_unaligned)) {
+ kfree(ring->vaddr_unaligned);
+ ring->vaddr_unaligned = NULL;
+ return -ENOMEM;
+ }
}
}

diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c
index 0a99aa7dd..7e4a8c820 100644
--- a/drivers/net/wireless/ath/ath11k/hal.c
+++ b/drivers/net/wireless/ath/ath11k/hal.c
@@ -628,15 +628,31 @@ u32 *ath11k_hal_srng_dst_peek(struct ath11k_base *ab, struct hal_srng *srng)
return NULL;
}

+static u32 *ath11k_hal_srng_dst_peek_with_dma(struct ath11k_base *ab,
+ struct hal_srng *srng, dma_addr_t *paddr)
+{
+ lockdep_assert_held(&srng->lock);
+
+ if (srng->u.dst_ring.tp != srng->u.dst_ring.cached_hp) {
+ *paddr = (srng->ring_base_paddr +
+ sizeof(*srng->ring_base_vaddr) * srng->u.dst_ring.tp);
+ return (srng->ring_base_vaddr + srng->u.dst_ring.tp);
+ }
+
+ return NULL;
+}
+
static void ath11k_hal_srng_prefetch_desc(struct ath11k_base *ab,
struct hal_srng *srng)
{
u32 *desc;
+ dma_addr_t desc_paddr;
+

/* prefetch only if desc is available */
- desc = ath11k_hal_srng_dst_peek(ab, srng);
+ desc = ath11k_hal_srng_dst_peek_with_dma(ab, srng, &desc_paddr);
if (likely(desc)) {
- dma_sync_single_for_cpu(ab->dev, virt_to_phys(desc),
+ dma_sync_single_for_cpu(ab->dev, desc_paddr,
(srng->entry_size * sizeof(u32)),
DMA_FROM_DEVICE);
prefetch(desc);


2024-01-09 15:41:53

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: ath11k: fix IOMMU errors on buffer rings

Zhenghao Gu <[email protected]> writes:

> virt_to_phys doesn't work on systems with IOMMU enabled,
> which have non-identity physical-to-IOVA mappings.

Can you give an example of such system? Just curious where you are
seeing this.

> It leads to IO_PAGE_FAULTs like this:
> [IO_PAGE_FAULT domain=0x0023 address=0x1cce00000 flags=0x0020]
> and no link can be established.

What do you mean with link in this context? Are you talking about 802.11
association?

> This patch changes that to dma_map_single(), which works correctly.

Good catch. And virt_to_phys() documentation even says this:

* This function does not give bus mappings for DMA transfers. In
* almost all conceivable cases a device driver should not be using
* this function

> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
> Signed-off-by: Zhenghao Gu <[email protected]>

Jeff, are you ok with this?

I did some cosmetics changes in the pending branch (removed unnecessary
parenthesis, reverse xmas tree etc), please check:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=fefa43d63e1928fce6e8c2bb626900e9ce98ca69

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

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

2024-01-09 17:05:35

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: ath11k: fix IOMMU errors on buffer rings

On 1/9/2024 7:41 AM, Kalle Valo wrote:
> Zhenghao Gu <[email protected]> writes:
>
>> virt_to_phys doesn't work on systems with IOMMU enabled,
>> which have non-identity physical-to-IOVA mappings.
>
> Can you give an example of such system? Just curious where you are
> seeing this.
>
>> It leads to IO_PAGE_FAULTs like this:
>> [IO_PAGE_FAULT domain=0x0023 address=0x1cce00000 flags=0x0020]
>> and no link can be established.
>
> What do you mean with link in this context? Are you talking about 802.11
> association?
>
>> This patch changes that to dma_map_single(), which works correctly.
>
> Good catch. And virt_to_phys() documentation even says this:
>
> * This function does not give bus mappings for DMA transfers. In
> * almost all conceivable cases a device driver should not be using
> * this function
>
>> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>> Signed-off-by: Zhenghao Gu <[email protected]>
>
> Jeff, are you ok with this?
>
> I did some cosmetics changes in the pending branch (removed unnecessary
> parenthesis, reverse xmas tree etc), please check:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=fefa43d63e1928fce6e8c2bb626900e9ce98ca69
>
LGTM, incorporates the v1 feedback from the engineering team

Acked-by: Jeff Johnson <[email protected]>

2024-01-10 00:43:35

by Zhenghao Gu

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: ath11k: fix IOMMU errors on buffer rings

On Tue, Jan 9, 2024 at 9:41 AM Kalle Valo <[email protected]> wrote:
>
> Zhenghao Gu <[email protected]> writes:
>
> > virt_to_phys doesn't work on systems with IOMMU enabled,
> > which have non-identity physical-to-IOVA mappings.
>
> Can you give an example of such system? Just curious where you are
> seeing this.

I'm testing on an AMD system with IOMMU enabled in the BIOS and "iommu=nopt"
in the kernel command line. I also noticed
https://bugzilla.kernel.org/show_bug.cgi?id=217056
which may be the same issue.

>
> > It leads to IO_PAGE_FAULTs like this:
> > [IO_PAGE_FAULT domain=0x0023 address=0x1cce00000 flags=0x0020]
> > and no link can be established.
>
> What do you mean with link in this context? Are you talking about 802.11
> association?

Yes.

>
> > This patch changes that to dma_map_single(), which works correctly.
>
> Good catch. And virt_to_phys() documentation even says this:
>
> * This function does not give bus mappings for DMA transfers. In
> * almost all conceivable cases a device driver should not be using
> * this function
>
> > Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
> > Signed-off-by: Zhenghao Gu <[email protected]>
>
> Jeff, are you ok with this?
>
> I did some cosmetics changes in the pending branch (removed unnecessary
> parenthesis, reverse xmas tree etc), please check:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=fefa43d63e1928fce6e8c2bb626900e9ce98ca69

2024-01-11 11:23:18

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: ath11k: fix IOMMU errors on buffer rings

Zhenghao Gu <[email protected]> wrote:

> virt_to_phys() doesn't work on systems with IOMMU enabled, which have
> non-identity physical-to-IOVA mappings. It leads to IO_PAGE_FAULTs like this:
>
> [IO_PAGE_FAULT domain=0x0023 address=0x1cce00000 flags=0x0020]
>
> And no association to the AP can be established.
>
> This patch changes that to dma_map_single(), which works correctly. Even
> virt_to_phys() documentation says device drivers should not use it:
>
> This function does not give bus mappings for DMA transfers. In
> almost all conceivable cases a device driver should not be using
> this function
>
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Zhenghao Gu <[email protected]>
> Acked-by: Jeff Johnson <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

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

aaf244141ed7 wifi: ath11k: fix IOMMU errors on buffer rings

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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