| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6538f66a-bc3e-6f16-180a-50ef37b02e98@leemhuis.info>
Date: Mon, 9 Jan 2023 08:32:17 +0100
From: "Linux kernel regression tracking (Thorsten Leemhuis)"
<regressions@...mhuis.info>
To: Felix Fietkau <nbd@....name>,
Mikhail Gavrilov <mikhail.v.gavrilov@...il.com>
Cc: lorenzo@...nel.org, sujuan.chen@...iatek.com,
Linux List Kernel Mailing <linux-wireless@...r.kernel.org>,
Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Subject: Re: [6.2][regression] after commit
cd372b8c99c5a5cf6a464acebb7e4a79af7ec8ae stopping working wifi mt7921e
On 04.01.23 15:20, Thorsten Leemhuis wrote:
> Hi, this is your Linux kernel regression tracker. Top-posting for once,
> to make this easily accessible to everyone.
>
> Felix, Lorenzo, did below fix for the regression
There is another report about an issue with mediatek wifi in 6.2-rc:
https://bugzilla.kernel.org/show_bug.cgi?id=216901
To me this looks like a duplicate of the report that started this thread.
(side note: there was another, earlier report that might be a dupe, too:
https://bugzilla.kernel.org/show_bug.cgi?id=216829 )
> Mikhail reported make
> any progress to get mainlined? It doesn't look like it from here, but I
> suspect I missed something, that's why I'm asking.
No reply. :-((
That lack of feedback is another reason why I'm CCing the network
maintainers now, as the mediatek wifi issues in 6.2-rc (this one) and
6.1 ([1]) are already hitting a nerve here because the fixes are
progressing so slowly. I known, it was holiday season, but seems quite a
few people ran into these regressions already, hence we IMHO should
really try to aim fixing both this week.
[1] see
https://lore.kernel.org/all/ac023262-c6cb-01ad-aeee-2dbf379f4c37@leemhuis.info/
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
> On 21.12.22 18:17, Felix Fietkau wrote:
>> On 21.12.22 17:46, Mikhail Gavrilov wrote:
>>> On Wed, Dec 21, 2022 at 7:12 PM Felix Fietkau <nbd@....name> wrote:
>>>>
>>>> Thanks! I guess I focused on the wrong part of your kernel log
>>>> initially. After more code review, I found that there is in fact a DMA
>>>> related bug in the commit that your bisection pointed to, which happened
>>>> to uncover and trigger the deadlock fixed by my other patch.
>>>>
>>>> So here's my fix for the DMA issue:
>>>> ---
>>> [cutted]
>>>> qbuf.skip_unmap = false;
>>>> - if (mt76_dma_add_buf(dev, q, &qbuf, 1, 0, buf, t) < 0) {
>>>> + if (mt76_dma_add_rx_buf(dev, q, &qbuf, buf) < 0) {
>>>> dma_unmap_single(dev->dma_dev, addr, len,
>>>> DMA_FROM_DEVICE);
>>>> skb_free_frag(buf);
>>>>
>>>
>>> Sorry for stupid question.
>>>
>>> Do you have a separate branch?
>>> I see that the code is differ between master branch and the patch.
>>>
>>> For example in patch the line:
>>> - if (mt76_dma_add_buf(dev, q, &qbuf, 1, 0, buf, t) < 0) {
>>> replaced by the line:
>>> + if (mt76_dma_add_rx_buf(dev, q, &qbuf, buf) < 0) {
>>>
>>> But in master branch
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/mediatek/mt76/dma.c?id=b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf#n604
>>> after line:
>>> qbuf.skip_unmap = false;
>>> followed the line:
>>> mt76_dma_add_buf(dev, q, &qbuf, 1, 0, buf, t);
>>> without if condition.
>>>
>>> So I'm stuck applying the patch :(
>> Sorry, I worked on a tree that had other pending fixes applied.
>> Please try this:
>>
>>
>> --- a/drivers/net/wireless/mediatek/mt76/dma.c
>> +++ b/drivers/net/wireless/mediatek/mt76/dma.c
>> @@ -205,6 +205,52 @@ mt76_dma_queue_reset(struct mt76_dev *dev, struct
>> mt76_queue *q)
>> mt76_dma_sync_idx(dev, q);
>> }
>>
>> +static int
>> +mt76_dma_add_rx_buf(struct mt76_dev *dev, struct mt76_queue *q,
>> + struct mt76_queue_buf *buf, void *data)
>> +{
>> + struct mt76_desc *desc = &q->desc[q->head];
>> + struct mt76_queue_entry *entry = &q->entry[q->head];
>> + struct mt76_txwi_cache *txwi = NULL;
>> + u32 buf1 = 0, ctrl;
>> + int idx = q->head;
>> + int rx_token;
>> +
>> + ctrl = FIELD_PREP(MT_DMA_CTL_SD_LEN0, buf[0].len);
>> +
>> + if ((q->flags & MT_QFLAG_WED) &&
>> + FIELD_GET(MT_QFLAG_WED_TYPE, q->flags) == MT76_WED_Q_RX) {
>> + txwi = mt76_get_rxwi(dev);
>> + if (!txwi)
>> + return -ENOMEM;
>> +
>> + rx_token = mt76_rx_token_consume(dev, data, txwi, buf->addr);
>> + if (rx_token < 0) {
>> + mt76_put_rxwi(dev, txwi);
>> + return -ENOMEM;
>> + }
>> +
>> + buf1 |= FIELD_PREP(MT_DMA_CTL_TOKEN, rx_token);
>> + ctrl |= MT_DMA_CTL_TO_HOST;
>> + }
>> +
>> + WRITE_ONCE(desc->buf0, cpu_to_le32(buf->addr));
>> + WRITE_ONCE(desc->buf1, cpu_to_le32(buf1));
>> + WRITE_ONCE(desc->ctrl, cpu_to_le32(ctrl));
>> + WRITE_ONCE(desc->info, 0);
>> +
>> + entry->dma_addr[0] = buf->addr;
>> + entry->dma_len[0] = buf->len;
>> + entry->txwi = txwi;
>> + entry->buf = data;
>> + entry->wcid = 0xffff;
>> + entry->skip_buf1 = true;
>> + q->head = (q->head + 1) % q->ndesc;
>> + q->queued++;
>> +
>> + return idx;
>> +}
>> +
>> static int
>> mt76_dma_add_buf(struct mt76_dev *dev, struct mt76_queue *q,
>> struct mt76_queue_buf *buf, int nbufs, u32 info,
>> @@ -212,65 +258,51 @@ mt76_dma_add_buf(struct mt76_dev *dev, struct
>> mt76_queue *q,
>> {
>> struct mt76_queue_entry *entry;
>> struct mt76_desc *desc;
>> - u32 ctrl;
>> int i, idx = -1;
>> + u32 ctrl, next;
>> +
>> + if (txwi) {
>> + q->entry[q->head].txwi = DMA_DUMMY_DATA;
>> + q->entry[q->head].skip_buf0 = true;
>> + }
>>
>> for (i = 0; i < nbufs; i += 2, buf += 2) {
>> u32 buf0 = buf[0].addr, buf1 = 0;
>>
>> idx = q->head;
>> - q->head = (q->head + 1) % q->ndesc;
>> + next = (q->head + 1) % q->ndesc;
>>
>> desc = &q->desc[idx];
>> entry = &q->entry[idx];
>>
>> - if ((q->flags & MT_QFLAG_WED) &&
>> - FIELD_GET(MT_QFLAG_WED_TYPE, q->flags) == MT76_WED_Q_RX) {
>> - struct mt76_txwi_cache *t = txwi;
>> - int rx_token;
>> -
>> - if (!t)
>> - return -ENOMEM;
>> -
>> - rx_token = mt76_rx_token_consume(dev, (void *)skb, t,
>> - buf[0].addr);
>> - buf1 |= FIELD_PREP(MT_DMA_CTL_TOKEN, rx_token);
>> - ctrl = FIELD_PREP(MT_DMA_CTL_SD_LEN0, buf[0].len) |
>> - MT_DMA_CTL_TO_HOST;
>> - } else {
>> - if (txwi) {
>> - q->entry[q->head].txwi = DMA_DUMMY_DATA;
>> - q->entry[q->head].skip_buf0 = true;
>> - }
>> -
>> - if (buf[0].skip_unmap)
>> - entry->skip_buf0 = true;
>> - entry->skip_buf1 = i == nbufs - 1;
>> -
>> - entry->dma_addr[0] = buf[0].addr;
>> - entry->dma_len[0] = buf[0].len;
>> -
>> - ctrl = FIELD_PREP(MT_DMA_CTL_SD_LEN0, buf[0].len);
>> - if (i < nbufs - 1) {
>> - entry->dma_addr[1] = buf[1].addr;
>> - entry->dma_len[1] = buf[1].len;
>> - buf1 = buf[1].addr;
>> - ctrl |= FIELD_PREP(MT_DMA_CTL_SD_LEN1, buf[1].len);
>> - if (buf[1].skip_unmap)
>> - entry->skip_buf1 = true;
>> - }
>> -
>> - if (i == nbufs - 1)
>> - ctrl |= MT_DMA_CTL_LAST_SEC0;
>> - else if (i == nbufs - 2)
>> - ctrl |= MT_DMA_CTL_LAST_SEC1;
>> + if (buf[0].skip_unmap)
>> + entry->skip_buf0 = true;
>> + entry->skip_buf1 = i == nbufs - 1;
>> +
>> + entry->dma_addr[0] = buf[0].addr;
>> + entry->dma_len[0] = buf[0].len;
>> +
>> + ctrl = FIELD_PREP(MT_DMA_CTL_SD_LEN0, buf[0].len);
>> + if (i < nbufs - 1) {
>> + entry->dma_addr[1] = buf[1].addr;
>> + entry->dma_len[1] = buf[1].len;
>> + buf1 = buf[1].addr;
>> + ctrl |= FIELD_PREP(MT_DMA_CTL_SD_LEN1, buf[1].len);
>> + if (buf[1].skip_unmap)
>> + entry->skip_buf1 = true;
>> }
>>
>> + if (i == nbufs - 1)
>> + ctrl |= MT_DMA_CTL_LAST_SEC0;
>> + else if (i == nbufs - 2)
>> + ctrl |= MT_DMA_CTL_LAST_SEC1;
>> +
>> WRITE_ONCE(desc->buf0, cpu_to_le32(buf0));
>> WRITE_ONCE(desc->buf1, cpu_to_le32(buf1));
>> WRITE_ONCE(desc->info, cpu_to_le32(info));
>> WRITE_ONCE(desc->ctrl, cpu_to_le32(ctrl));
>>
>> + q->head = next;
>> q->queued++;
>> }
>>
>> @@ -577,17 +609,9 @@ mt76_dma_rx_fill(struct mt76_dev *dev, struct
>> mt76_queue *q)
>> spin_lock_bh(&q->lock);
>>
>> while (q->queued < q->ndesc - 1) {
>> - struct mt76_txwi_cache *t = NULL;
>> struct mt76_queue_buf qbuf;
>> void *buf = NULL;
>>
>> - if ((q->flags & MT_QFLAG_WED) &&
>> - FIELD_GET(MT_QFLAG_WED_TYPE, q->flags) == MT76_WED_Q_RX) {
>> - t = mt76_get_rxwi(dev);
>> - if (!t)
>> - break;
>> - }
>> -
>> buf = page_frag_alloc(rx_page, q->buf_size, GFP_ATOMIC);
>> if (!buf)
>> break;
>> @@ -601,7 +625,12 @@ mt76_dma_rx_fill(struct mt76_dev *dev, struct
>> mt76_queue *q)
>> qbuf.addr = addr + offset;
>> qbuf.len = len - offset;
>> qbuf.skip_unmap = false;
>> - mt76_dma_add_buf(dev, q, &qbuf, 1, 0, buf, t);
>> + if (mt76_dma_add_rx_buf(dev, q, &qbuf, buf) < 0) {
>> + dma_unmap_single(dev->dma_dev, addr, len,
>> + DMA_FROM_DEVICE);
>> + skb_free_frag(buf);
>> + break;
>> + }
>> frames++;
>> }
>>
>>
>> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
>> @@ -653,6 +653,13 @@ static u32 mt7915_mmio_wed_init_rx_buf(struct
>> mtk_wed_device *wed, int size)
>>
>> desc->buf0 = cpu_to_le32(phy_addr);
>> token = mt76_rx_token_consume(&dev->mt76, ptr, t, phy_addr);
>> + if (token < 0) {
>> + dma_unmap_single(dev->mt76.dma_dev, phy_addr,
>> + wed->wlan.rx_size, DMA_TO_DEVICE);
>> + skb_free_frag(ptr);
>> + goto unmap;
>> + }
>> +
>> desc->token |= cpu_to_le32(FIELD_PREP(MT_DMA_CTL_TOKEN,
>> token));
>> desc++;
>>
>> --- a/drivers/net/wireless/mediatek/mt76/tx.c
>> +++ b/drivers/net/wireless/mediatek/mt76/tx.c
>> @@ -764,11 +764,12 @@ int mt76_rx_token_consume(struct mt76_dev *dev,
>> void *ptr,
>> spin_lock_bh(&dev->rx_token_lock);
>> token = idr_alloc(&dev->rx_token, t, 0, dev->rx_token_size,
>> GFP_ATOMIC);
>> + if (token >= 0) {
>> + t->ptr = ptr;
>> + t->dma_addr = phys;
>> + }
>> spin_unlock_bh(&dev->rx_token_lock);
>>
>> - t->ptr = ptr;
>> - t->dma_addr = phys;
>> -
>> return token;
>> }
>> EXPORT_SYMBOL_GPL(mt76_rx_token_consume);
>>
Powered by blists - more mailing lists