* [PATCH] net: cpsw: invalidate complete buffer
@ 2015-02-27 9:56 Jan Weitzel
2015-02-27 11:47 ` Lucas Stach
0 siblings, 1 reply; 6+ messages in thread
From: Jan Weitzel @ 2015-02-27 9:56 UTC (permalink / raw)
To: barebox
Without invalidating the complete buffer before giving it to
dma_inv_range, we got strange packets.
Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>
Tested-by: Teresa Gámez <t.gamez@phytec.de>
---
drivers/net/cpsw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 799fac8..33afdc3 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -886,7 +886,7 @@ static int cpsw_recv(struct eth_device *edev)
int len;
while (cpdma_process(priv, &priv->rx_chan, &buffer, &len) >= 0) {
- dma_inv_range((ulong)buffer, (ulong)buffer + len);
+ dma_inv_range((ulong)buffer, (ulong)buffer + PKTSIZE);
net_receive(edev, buffer, len);
cpdma_submit(priv, &priv->rx_chan, buffer, PKTSIZE);
}
--
1.9.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: cpsw: invalidate complete buffer
2015-02-27 9:56 [PATCH] net: cpsw: invalidate complete buffer Jan Weitzel
@ 2015-02-27 11:47 ` Lucas Stach
2015-02-27 13:14 ` Jan Weitzel
0 siblings, 1 reply; 6+ messages in thread
From: Lucas Stach @ 2015-02-27 11:47 UTC (permalink / raw)
To: Jan Weitzel; +Cc: barebox
Am Freitag, den 27.02.2015, 10:56 +0100 schrieb Jan Weitzel:
> Without invalidating the complete buffer before giving it to
> dma_inv_range, we got strange packets.
>
This is most likely not the correct fix. If this helps then our
dma_inv_range functions aren't working properly, which would be really
bad. How do those "strange packets" look like?
> Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>
> Tested-by: Teresa Gámez <t.gamez@phytec.de>
> ---
> drivers/net/cpsw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
> index 799fac8..33afdc3 100644
> --- a/drivers/net/cpsw.c
> +++ b/drivers/net/cpsw.c
> @@ -886,7 +886,7 @@ static int cpsw_recv(struct eth_device *edev)
> int len;
>
> while (cpdma_process(priv, &priv->rx_chan, &buffer, &len) >= 0) {
> - dma_inv_range((ulong)buffer, (ulong)buffer + len);
> + dma_inv_range((ulong)buffer, (ulong)buffer + PKTSIZE);
> net_receive(edev, buffer, len);
> cpdma_submit(priv, &priv->rx_chan, buffer, PKTSIZE);
> }
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: cpsw: invalidate complete buffer
2015-02-27 11:47 ` Lucas Stach
@ 2015-02-27 13:14 ` Jan Weitzel
2015-02-27 13:39 ` Lucas Stach
0 siblings, 1 reply; 6+ messages in thread
From: Jan Weitzel @ 2015-02-27 13:14 UTC (permalink / raw)
To: Lucas Stach; +Cc: barebox
On Fri, Feb 27, 2015 at 12:47:24PM +0100, Lucas Stach wrote:
> Am Freitag, den 27.02.2015, 10:56 +0100 schrieb Jan Weitzel:
> > Without invalidating the complete buffer before giving it to
> > dma_inv_range, we got strange packets.
> >
>
> This is most likely not the correct fix. If this helps then our
> dma_inv_range functions aren't working properly, which would be really
> bad. How do those "strange packets" look like?
>
We saw the problem with tftp transfers of a 76 byte image. Debug print in
tftp_handler
good:
tftp_handler: len:76 blocksize:1432 to fifo
00000000: 06 08 ba de 7d 27 e1 2c 52 2f cf 77 e0 d3 23 da ....}'.,R/.w..#.
00000010: 62 d4 42 3e 03 20 3a c2 51 aa 51 15 73 be 9e 21 b.B>. :.Q.Q.s..!
00000020: 03 ac 78 a9 e3 4f cd 4d 6d ba 93 fe 83 dc fe 82 ..x..O.Mm.......
00000030: bb f8 24 29 6e 6f 53 1c 91 52 4b 77 1b 72 ff a0 ..$)noS..RKw.r..
00000040: 5b 98 1c 20 28 09 0f 4c 93 3c 22 08 [.. (..L.<".
bad:
tftp_handler: len:76 blocksize:1432 to fifo:
00000000: 06 08 ba de 7d 27 e1 2c 52 2f cf 77 e0 d3 23 da ....}'.,R/.w..#.
00000010: 62 d4 6b 73 69 7a 65 00 31 34 33 32 00 b0 1b d1 b.ksize.1432....
00000020: 5b d4 78 a9 e3 4f cd 4d 6d ba 93 fe 83 dc fe 82 [.x..O.Mm.......
00000030: bb f8 24 29 6e 6f 53 1c 91 52 4b 77 1b 72 ff a0 ..$)noS..RKw.r..
00000040: 5b 98 1c 20 28 09 0f 4c 93 3c 22 08 [.. (..L.<".
The ethernet package has 122 Bytes and the error in the received file is on
offset 18. The data "b.ksize.1432" comes also from tftp packets.
Jan
> > Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>
> > Tested-by: Teresa Gámez <t.gamez@phytec.de>
> > ---
> > drivers/net/cpsw.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
> > index 799fac8..33afdc3 100644
> > --- a/drivers/net/cpsw.c
> > +++ b/drivers/net/cpsw.c
> > @@ -886,7 +886,7 @@ static int cpsw_recv(struct eth_device *edev)
> > int len;
> >
> > while (cpdma_process(priv, &priv->rx_chan, &buffer, &len) >= 0) {
> > - dma_inv_range((ulong)buffer, (ulong)buffer + len);
> > + dma_inv_range((ulong)buffer, (ulong)buffer + PKTSIZE);
> > net_receive(edev, buffer, len);
> > cpdma_submit(priv, &priv->rx_chan, buffer, PKTSIZE);
> > }
>
> --
> Pengutronix e.K. | Lucas Stach |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
>
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: cpsw: invalidate complete buffer
2015-02-27 13:14 ` Jan Weitzel
@ 2015-02-27 13:39 ` Lucas Stach
2015-02-27 13:45 ` Lucas Stach
0 siblings, 1 reply; 6+ messages in thread
From: Lucas Stach @ 2015-02-27 13:39 UTC (permalink / raw)
To: Jan Weitzel; +Cc: barebox
Am Freitag, den 27.02.2015, 14:14 +0100 schrieb Jan Weitzel:
> On Fri, Feb 27, 2015 at 12:47:24PM +0100, Lucas Stach wrote:
> > Am Freitag, den 27.02.2015, 10:56 +0100 schrieb Jan Weitzel:
> > > Without invalidating the complete buffer before giving it to
> > > dma_inv_range, we got strange packets.
> > >
> >
> > This is most likely not the correct fix. If this helps then our
> > dma_inv_range functions aren't working properly, which would be really
> > bad. How do those "strange packets" look like?
> >
> We saw the problem with tftp transfers of a 76 byte image. Debug print in
> tftp_handler
>
> good:
> tftp_handler: len:76 blocksize:1432 to fifo
> 00000000: 06 08 ba de 7d 27 e1 2c 52 2f cf 77 e0 d3 23 da ....}'.,R/.w..#.
> 00000010: 62 d4 42 3e 03 20 3a c2 51 aa 51 15 73 be 9e 21 b.B>. :.Q.Q.s..!
> 00000020: 03 ac 78 a9 e3 4f cd 4d 6d ba 93 fe 83 dc fe 82 ..x..O.Mm.......
> 00000030: bb f8 24 29 6e 6f 53 1c 91 52 4b 77 1b 72 ff a0 ..$)noS..RKw.r..
> 00000040: 5b 98 1c 20 28 09 0f 4c 93 3c 22 08 [.. (..L.<".
>
> bad:
> tftp_handler: len:76 blocksize:1432 to fifo:
> 00000000: 06 08 ba de 7d 27 e1 2c 52 2f cf 77 e0 d3 23 da ....}'.,R/.w..#.
> 00000010: 62 d4 6b 73 69 7a 65 00 31 34 33 32 00 b0 1b d1 b.ksize.1432....
> 00000020: 5b d4 78 a9 e3 4f cd 4d 6d ba 93 fe 83 dc fe 82 [.x..O.Mm.......
> 00000030: bb f8 24 29 6e 6f 53 1c 91 52 4b 77 1b 72 ff a0 ..$)noS..RKw.r..
> 00000040: 5b 98 1c 20 28 09 0f 4c 93 3c 22 08 [.. (..L.<".
>
>
> The ethernet package has 122 Bytes and the error in the received file is on
> offset 18. The data "b.ksize.1432" comes also from tftp packets.
>
This doesn't look like a problem to invalidate the cache, but more like
writeback of old cachelines while the hardware owns the buffer. Can you
try the series "Phasing out direct usage of asm/mmu.h on ARM" and see if
this still happens there? If I'm correct this series should fix this
problem.
I'll send an updated version of this series in the evening, but you
should be able to correct the typo in cpsw.c yourself for testing. :)
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: cpsw: invalidate complete buffer
2015-02-27 13:39 ` Lucas Stach
@ 2015-02-27 13:45 ` Lucas Stach
2015-03-02 7:35 ` Jan Weitzel
0 siblings, 1 reply; 6+ messages in thread
From: Lucas Stach @ 2015-02-27 13:45 UTC (permalink / raw)
To: Jan Weitzel; +Cc: barebox
Am Freitag, den 27.02.2015, 14:39 +0100 schrieb Lucas Stach:
> Am Freitag, den 27.02.2015, 14:14 +0100 schrieb Jan Weitzel:
> > On Fri, Feb 27, 2015 at 12:47:24PM +0100, Lucas Stach wrote:
> > > Am Freitag, den 27.02.2015, 10:56 +0100 schrieb Jan Weitzel:
> > > > Without invalidating the complete buffer before giving it to
> > > > dma_inv_range, we got strange packets.
> > > >
> > >
> > > This is most likely not the correct fix. If this helps then our
> > > dma_inv_range functions aren't working properly, which would be really
> > > bad. How do those "strange packets" look like?
> > >
> > We saw the problem with tftp transfers of a 76 byte image. Debug print in
> > tftp_handler
> >
> > good:
> > tftp_handler: len:76 blocksize:1432 to fifo
> > 00000000: 06 08 ba de 7d 27 e1 2c 52 2f cf 77 e0 d3 23 da ....}'.,R/.w..#.
> > 00000010: 62 d4 42 3e 03 20 3a c2 51 aa 51 15 73 be 9e 21 b.B>. :.Q.Q.s..!
> > 00000020: 03 ac 78 a9 e3 4f cd 4d 6d ba 93 fe 83 dc fe 82 ..x..O.Mm.......
> > 00000030: bb f8 24 29 6e 6f 53 1c 91 52 4b 77 1b 72 ff a0 ..$)noS..RKw.r..
> > 00000040: 5b 98 1c 20 28 09 0f 4c 93 3c 22 08 [.. (..L.<".
> >
> > bad:
> > tftp_handler: len:76 blocksize:1432 to fifo:
> > 00000000: 06 08 ba de 7d 27 e1 2c 52 2f cf 77 e0 d3 23 da ....}'.,R/.w..#.
> > 00000010: 62 d4 6b 73 69 7a 65 00 31 34 33 32 00 b0 1b d1 b.ksize.1432....
> > 00000020: 5b d4 78 a9 e3 4f cd 4d 6d ba 93 fe 83 dc fe 82 [.x..O.Mm.......
> > 00000030: bb f8 24 29 6e 6f 53 1c 91 52 4b 77 1b 72 ff a0 ..$)noS..RKw.r..
> > 00000040: 5b 98 1c 20 28 09 0f 4c 93 3c 22 08 [.. (..L.<".
> >
> >
> > The ethernet package has 122 Bytes and the error in the received file is on
> > offset 18. The data "b.ksize.1432" comes also from tftp packets.
> >
> This doesn't look like a problem to invalidate the cache, but more like
> writeback of old cachelines while the hardware owns the buffer. Can you
> try the series "Phasing out direct usage of asm/mmu.h on ARM" and see if
> this still happens there? If I'm correct this series should fix this
> problem.
>
> I'll send an updated version of this series in the evening, but you
> should be able to correct the typo in cpsw.c yourself for testing. :)
Or try this patch, which should do the same as the new cache handling,
but may be acceptable for master.
------------------------>8--------------------------------------------
From f96aec8bd87ac29247617bdcfab41048b942e899 Mon Sep 17 00:00:00 2001
From: Lucas Stach <l.stach@pengutronix.de>
Date: Fri, 27 Feb 2015 14:41:46 +0100
Subject: [PATCH] net: cpsw: prevent stray cache writeback
The cache should be invalidated when transfering ownership of a buffer
to the device. Otherwise the writeback of dirty cache lines can
corrupt the hardware written data.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/net/cpsw.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 799fac89a2f3..301b8a9dfde5 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -888,6 +888,7 @@ static int cpsw_recv(struct eth_device *edev)
while (cpdma_process(priv, &priv->rx_chan, &buffer, &len) >= 0) {
dma_inv_range((ulong)buffer, (ulong)buffer + len);
net_receive(edev, buffer, len);
+ dma_inv_range((ulong)buffer, (ulong)buffer + len);
cpdma_submit(priv, &priv->rx_chan, buffer, PKTSIZE);
}
--
2.1.4
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: cpsw: invalidate complete buffer
2015-02-27 13:45 ` Lucas Stach
@ 2015-03-02 7:35 ` Jan Weitzel
0 siblings, 0 replies; 6+ messages in thread
From: Jan Weitzel @ 2015-03-02 7:35 UTC (permalink / raw)
To: Lucas Stach; +Cc: barebox
On Fri, Feb 27, 2015 at 02:45:18PM +0100, Lucas Stach wrote:
> Am Freitag, den 27.02.2015, 14:39 +0100 schrieb Lucas Stach:
> > Am Freitag, den 27.02.2015, 14:14 +0100 schrieb Jan Weitzel:
> > > On Fri, Feb 27, 2015 at 12:47:24PM +0100, Lucas Stach wrote:
> > > > Am Freitag, den 27.02.2015, 10:56 +0100 schrieb Jan Weitzel:
> > > > > Without invalidating the complete buffer before giving it to
> > > > > dma_inv_range, we got strange packets.
> > > > >
> > > >
> > > > This is most likely not the correct fix. If this helps then our
> > > > dma_inv_range functions aren't working properly, which would be really
> > > > bad. How do those "strange packets" look like?
> > > >
> > > We saw the problem with tftp transfers of a 76 byte image. Debug print in
> > > tftp_handler
> > >
> > > good:
> > > tftp_handler: len:76 blocksize:1432 to fifo
> > > 00000000: 06 08 ba de 7d 27 e1 2c 52 2f cf 77 e0 d3 23 da ....}'.,R/.w..#.
> > > 00000010: 62 d4 42 3e 03 20 3a c2 51 aa 51 15 73 be 9e 21 b.B>. :.Q.Q.s..!
> > > 00000020: 03 ac 78 a9 e3 4f cd 4d 6d ba 93 fe 83 dc fe 82 ..x..O.Mm.......
> > > 00000030: bb f8 24 29 6e 6f 53 1c 91 52 4b 77 1b 72 ff a0 ..$)noS..RKw.r..
> > > 00000040: 5b 98 1c 20 28 09 0f 4c 93 3c 22 08 [.. (..L.<".
> > >
> > > bad:
> > > tftp_handler: len:76 blocksize:1432 to fifo:
> > > 00000000: 06 08 ba de 7d 27 e1 2c 52 2f cf 77 e0 d3 23 da ....}'.,R/.w..#.
> > > 00000010: 62 d4 6b 73 69 7a 65 00 31 34 33 32 00 b0 1b d1 b.ksize.1432....
> > > 00000020: 5b d4 78 a9 e3 4f cd 4d 6d ba 93 fe 83 dc fe 82 [.x..O.Mm.......
> > > 00000030: bb f8 24 29 6e 6f 53 1c 91 52 4b 77 1b 72 ff a0 ..$)noS..RKw.r..
> > > 00000040: 5b 98 1c 20 28 09 0f 4c 93 3c 22 08 [.. (..L.<".
> > >
> > >
> > > The ethernet package has 122 Bytes and the error in the received file is on
> > > offset 18. The data "b.ksize.1432" comes also from tftp packets.
> > >
> > This doesn't look like a problem to invalidate the cache, but more like
> > writeback of old cachelines while the hardware owns the buffer. Can you
> > try the series "Phasing out direct usage of asm/mmu.h on ARM" and see if
> > this still happens there? If I'm correct this series should fix this
> > problem.
> >
> > I'll send an updated version of this series in the evening, but you
> > should be able to correct the typo in cpsw.c yourself for testing. :)
>
> Or try this patch, which should do the same as the new cache handling,
> but may be acceptable for master.
Your patch works and invalidating after reading makes sense ;)
Can we take this for master? I'll test the series.
Tested-by: Jan Weitzel <j.weitzel@phytec.de>
>
> ------------------------>8--------------------------------------------
> From f96aec8bd87ac29247617bdcfab41048b942e899 Mon Sep 17 00:00:00 2001
> From: Lucas Stach <l.stach@pengutronix.de>
> Date: Fri, 27 Feb 2015 14:41:46 +0100
> Subject: [PATCH] net: cpsw: prevent stray cache writeback
>
> The cache should be invalidated when transfering ownership of a buffer
> to the device. Otherwise the writeback of dirty cache lines can
> corrupt the hardware written data.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/net/cpsw.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
> index 799fac89a2f3..301b8a9dfde5 100644
> --- a/drivers/net/cpsw.c
> +++ b/drivers/net/cpsw.c
> @@ -888,6 +888,7 @@ static int cpsw_recv(struct eth_device *edev)
> while (cpdma_process(priv, &priv->rx_chan, &buffer, &len) >= 0) {
> dma_inv_range((ulong)buffer, (ulong)buffer + len);
> net_receive(edev, buffer, len);
> + dma_inv_range((ulong)buffer, (ulong)buffer + len);
> cpdma_submit(priv, &priv->rx_chan, buffer, PKTSIZE);
> }
>
> --
> 2.1.4
> --
> Pengutronix e.K. | Lucas Stach |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
>
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-02 7:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-27 9:56 [PATCH] net: cpsw: invalidate complete buffer Jan Weitzel
2015-02-27 11:47 ` Lucas Stach
2015-02-27 13:14 ` Jan Weitzel
2015-02-27 13:39 ` Lucas Stach
2015-02-27 13:45 ` Lucas Stach
2015-03-02 7:35 ` Jan Weitzel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox