* [RFC PATCH 2/4] b44: Fix off-by-one error in acceptable address range
@ 2019-12-08 8:36 George Spelvin
2019-12-09 17:29 ` Michael Chan
0 siblings, 1 reply; 3+ messages in thread
From: George Spelvin @ 2019-12-08 8:36 UTC (permalink / raw)
To: michael.chan, netdev; +Cc: hauke, lkml
The requirement is dma_addr + size <= 0x40000000, not 0x3fffffff.
In a logically separate but overlapping change, this patch also
rearranges the logic for detecting failures to use a goto rather
than testing dma_mapping_error() twice. The latter is expensive if
CONFIG_DMA_API_DEBUG is set, but also for bug-proofing reasons I try to
avoid having the same condition in two places that must be kept in sync.
Signed-off-by: George Spelvin <lkml@sdf.org>
---
drivers/net/ethernet/broadcom/b44.c | 42 ++++++++++++++++-------------
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 394671230c1c..e540d5646aef 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -680,12 +680,13 @@ static int b44_alloc_rx_skb(struct b44 *bp, int src_idx, u32 dest_idx_unmasked)
/* Hardware bug work-around, the chip is unable to do PCI DMA
to/from anything above 1GB :-( */
- if (dma_mapping_error(bp->sdev->dma_dev, mapping) ||
- mapping + RX_PKT_BUF_SZ > DMA_BIT_MASK(30)) {
+ if (dma_mapping_error(bp->sdev->dma_dev, mapping))
+ goto workaround;
+ if (mapping + RX_PKT_BUF_SZ > DMA_BIT_MASK(30)+1) {
/* Sigh... */
- if (!dma_mapping_error(bp->sdev->dma_dev, mapping))
- dma_unmap_single(bp->sdev->dma_dev, mapping,
- RX_PKT_BUF_SZ, DMA_FROM_DEVICE);
+ dma_unmap_single(bp->sdev->dma_dev, mapping,
+ RX_PKT_BUF_SZ, DMA_FROM_DEVICE);
+workaround:
dev_kfree_skb_any(skb);
skb = alloc_skb(RX_PKT_BUF_SZ, GFP_ATOMIC | GFP_DMA);
if (skb == NULL)
@@ -693,10 +694,12 @@ static int b44_alloc_rx_skb(struct b44 *bp, int src_idx, u32 dest_idx_unmasked)
mapping = dma_map_single(bp->sdev->dma_dev, skb->data,
RX_PKT_BUF_SZ,
DMA_FROM_DEVICE);
- if (dma_mapping_error(bp->sdev->dma_dev, mapping) ||
- mapping + RX_PKT_BUF_SZ > DMA_BIT_MASK(30)) {
- if (!dma_mapping_error(bp->sdev->dma_dev, mapping))
- dma_unmap_single(bp->sdev->dma_dev, mapping, RX_PKT_BUF_SZ,DMA_FROM_DEVICE);
+ if (dma_mapping_error(bp->sdev->dma_dev, mapping))
+ goto failed;
+ if (mapping + RX_PKT_BUF_SZ > DMA_BIT_MASK(30)+1) {
+ dma_unmap_single(bp->sdev->dma_dev, mapping,
+ RX_PKT_BUF_SZ, DMA_FROM_DEVICE);
+failed:
dev_kfree_skb_any(skb);
return -ENOMEM;
}
@@ -990,24 +993,27 @@ static netdev_tx_t b44_start_xmit(struct sk_buff *skb, struct net_device *dev)
mapping = dma_map_single_attrs(bp->sdev->dma_dev, skb->data, len,
DMA_TO_DEVICE, DMA_ATTR_NO_WARN);
- if (dma_mapping_error(bp->sdev->dma_dev, mapping) || mapping + len > DMA_BIT_MASK(30)) {
+ if (dma_mapping_error(bp->sdev->dma_dev, mapping))
+ goto workaround;
+ if (mapping + len > DMA_BIT_MASK(30)+1) {
struct sk_buff *bounce_skb;
/* Chip can't handle DMA to/from >1GB, use bounce buffer */
- if (!dma_mapping_error(bp->sdev->dma_dev, mapping))
- dma_unmap_single(bp->sdev->dma_dev, mapping, len,
- DMA_TO_DEVICE);
-
+ dma_unmap_single(bp->sdev->dma_dev, mapping, len,
+ DMA_TO_DEVICE);
+workaround:
bounce_skb = alloc_skb(len, GFP_ATOMIC | GFP_DMA);
if (!bounce_skb)
goto err_out;
mapping = dma_map_single(bp->sdev->dma_dev, bounce_skb->data,
len, DMA_TO_DEVICE);
- if (dma_mapping_error(bp->sdev->dma_dev, mapping) || mapping + len > DMA_BIT_MASK(30)) {
- if (!dma_mapping_error(bp->sdev->dma_dev, mapping))
- dma_unmap_single(bp->sdev->dma_dev, mapping,
- len, DMA_TO_DEVICE);
+ if (dma_mapping_error(bp->sdev->dma_dev, mapping))
+ goto failed;
+ if (mapping + len > DMA_BIT_MASK(30)+1) {
+ dma_unmap_single(bp->sdev->dma_dev, mapping, len,
+ DMA_TO_DEVICE);
+failed:
dev_kfree_skb_any(bounce_skb);
goto err_out;
}
--
2.24.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC PATCH 2/4] b44: Fix off-by-one error in acceptable address range
2019-12-08 8:36 [RFC PATCH 2/4] b44: Fix off-by-one error in acceptable address range George Spelvin
@ 2019-12-09 17:29 ` Michael Chan
2019-12-09 18:01 ` George Spelvin
0 siblings, 1 reply; 3+ messages in thread
From: Michael Chan @ 2019-12-09 17:29 UTC (permalink / raw)
To: George Spelvin; +Cc: Netdev, Hauke Mehrtens
On Sun, Dec 8, 2019 at 12:36 AM George Spelvin <lkml@sdf.org> wrote:
>
> The requirement is dma_addr + size <= 0x40000000, not 0x3fffffff.
>
> In a logically separate but overlapping change, this patch also
> rearranges the logic for detecting failures to use a goto rather
> than testing dma_mapping_error() twice. The latter is expensive if
> CONFIG_DMA_API_DEBUG is set, but also for bug-proofing reasons I try to
> avoid having the same condition in two places that must be kept in sync.
>
> Signed-off-by: George Spelvin <lkml@sdf.org>
> ---
> drivers/net/ethernet/broadcom/b44.c | 42 ++++++++++++++++-------------
> 1 file changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
> index 394671230c1c..e540d5646aef 100644
> --- a/drivers/net/ethernet/broadcom/b44.c
> +++ b/drivers/net/ethernet/broadcom/b44.c
> @@ -680,12 +680,13 @@ static int b44_alloc_rx_skb(struct b44 *bp, int src_idx, u32 dest_idx_unmasked)
>
> /* Hardware bug work-around, the chip is unable to do PCI DMA
> to/from anything above 1GB :-( */
> - if (dma_mapping_error(bp->sdev->dma_dev, mapping) ||
> - mapping + RX_PKT_BUF_SZ > DMA_BIT_MASK(30)) {
> + if (dma_mapping_error(bp->sdev->dma_dev, mapping))
> + goto workaround;
> + if (mapping + RX_PKT_BUF_SZ > DMA_BIT_MASK(30)+1) {
The patchset looks ok to me. The only minor suggestion is to define
this (DMA_BIT_MASK(30) + 1) as B44_DMA_ADDR_LIMIT or something like
that so you don't have to repeat it so many times.
Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH 2/4] b44: Fix off-by-one error in acceptable address range
2019-12-09 17:29 ` Michael Chan
@ 2019-12-09 18:01 ` George Spelvin
0 siblings, 0 replies; 3+ messages in thread
From: George Spelvin @ 2019-12-09 18:01 UTC (permalink / raw)
To: lkml, michael.chan; +Cc: hauke, netdev
From michael.chan@broadcom.com Mon Dec 9 17:30:02 2019
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=broadcom.com; s=google;
h=mime-version:references:in-reply-to:from:date:message-id:subject:to
:cc;
bh=Dcgdz44q2GCvADR7a/LkLs9ynpEQVQ92g/NDbUJ9ByU=;
b=PKOR9wiZIB18b/51jkUG/wXIFefbTZbsIoGHXMC3Wa+HrxHrvmOkKTzDzw+sXBVIlk
WIeKjcijz8em6sM32WwxvXsG6eJYecW1+Vr8KQUTsTvwAezJa3uxBtNCwH0rvj3Hax2z
2OapWK7fvk/DbmGvycFL+e8p61RygSe62KIF8=
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:mime-version:references:in-reply-to:from:date
:message-id:subject:to:cc;
bh=Dcgdz44q2GCvADR7a/LkLs9ynpEQVQ92g/NDbUJ9ByU=;
b=IFhkqkqmkV1immJLIPu0fZ4IWzVHMsSFers4GqLqTxLaiKuXyQfvRhQSRM37NYwaON
IE/0JjtXxZyECNyF1M2sFl/VVo4nVQG28mHeuJK1GG8WtToN+bF40KU+3eGhDWFgJYGK
XfZjSIrHORSGYlKRWdpfv+jJizDt0/zB1RvMipk9BSW3mp5MLNHB8gnWmqFpptDdkqYX
4R906dAil3MORLKrH33jpnHlWdZeTPc++n4+b+YhAggBzLCifmfcNP5Gsi9MVV6+hzot
8RxP7p+m1839v84KhyhUAeuJ64QLKaQA9jAMF9IOU/N1DJCFLFE6rPmh0JM5jkl82cDp
4qew==
X-Gm-Message-State: APjAAAWuTa/gVp+rOdJT+15VVbVrsYIOYBlX4a4u8GUmZNE3mGqVmIfL
8BYFLElwhyjGzOgl3smgAsybRD66b+oZQTZLOuyTSOxH
X-Google-Smtp-Source: APXvYqz4AY07BzHgic5er69wv22sNZkWuNoimJ4dj0xiy8faF2rZTCRbn2c2dvhbKtnKoCqZ27uqaKVyNe2AUdEl/0o=
X-Received: by 2002:a05:6830:1e75:: with SMTP id m21mr21646882otr.36.1575912597048;
Mon, 09 Dec 2019 09:29:57 -0800 (PST)
MIME-Version: 1.0
References: <201912080836.xB88amHd015549@sdf.org>
In-Reply-To: <201912080836.xB88amHd015549@sdf.org>
From: Michael Chan <michael.chan@broadcom.com>
Date: Mon, 9 Dec 2019 09:29:46 -0800
Subject: Re: [RFC PATCH 2/4] b44: Fix off-by-one error in acceptable address range
To: George Spelvin <lkml@sdf.org>
Cc: Netdev <netdev@vger.kernel.org>, Hauke Mehrtens <hauke@hauke-m.de>
Content-Type: text/plain; charset="UTF-8"
On Mon, 9 Dec 2019 at 09:29:46 Michael Chan wrote:
> The patchset looks ok to me. The only minor suggestion is to define
> this (DMA_BIT_MASK(30) + 1) as B44_DMA_ADDR_LIMIT or something like
> that so you don't have to repeat it so many times.
Thank you for the prompt reply. Yes, that's a sensible thing to do.
There's an existing constant I can use: SSB_PCI_DMA_SZ.
Do you have any idea about the WARN_ON_ONCE in report_addr()? That's
a someone more invasive issue. E.g. should it be changed to also
honour the NO_WARN flag?
Looking at the 440x datasheet, one significantly more ambitious
thing I wonder about is if it would be possible to map the swiotlb
using SBToPCITranslation[01] and let that layer do the heavy lifting.
The translation registers are set up, but I don't see them ever actually
used for anything on these chips. (But I havnn't grokked all the
drivers/ssb code yet, so I might have missed something.)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-09 18:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-08 8:36 [RFC PATCH 2/4] b44: Fix off-by-one error in acceptable address range George Spelvin
2019-12-09 17:29 ` Michael Chan
2019-12-09 18:01 ` George Spelvin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).