[U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de --- drivers/net/zynq_gem.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..5825b6dd99 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -621,6 +621,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
+ flush_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN, ARCH_DMA_MINALIGN)); + barrier(); + return frame_len; }
On Thu, Dec 13, 2018 at 6:18 PM Stefan Theil stefan.theil@mixed-mode.de wrote:
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
drivers/net/zynq_gem.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..5825b6dd99 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -621,6 +621,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
flush_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
Shouldn't it be invalidate_dcache_range()?
barrier();return frame_len;}
Regards, Bin
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then. Also flushing the receive buffers in the transmit function makes no sense and can be removed.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
--- Changes for v2: - Use invalidate_dcache_range instead of flush_dcache_range Changes for v3: - Remove unnecessary flushing of all RX buffers in zynq_gem_send Changes for v4: - Invalidate receive buffers after allocating them in zynq_gem_probe --- drivers/net/zynq_gem.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..79a22fb1ed 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -570,11 +570,6 @@ static int zynq_gem_send(struct udevice *dev, void *ptr, int len) addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr + size); - - addr = (ulong)priv->rxbuffers; - addr &= ~(ARCH_DMA_MINALIGN - 1); - size = roundup((RX_BUF * PKTSIZE_ALIGN), ARCH_DMA_MINALIGN); - flush_dcache_range(addr, addr + size); barrier();
/* Start transmit */ @@ -621,6 +616,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
+ invalidate_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN, ARCH_DMA_MINALIGN)); + barrier(); + return frame_len; }
@@ -705,7 +703,9 @@ static int zynq_gem_probe(struct udevice *dev) if (!priv->rxbuffers) return -ENOMEM;
- memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN); + u32 addr = (ulong)priv->rxbuffers; + invalidate_dcache_range(addr, addr + roundup(RX_BUF * PKTSIZE_ALIGN, ARCH_DMA_MINALIGN)); + barrier();
/* Align bd_space to MMU_SECTION_SHIFT */ bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
Hi Stefan,
On Mon, Dec 17, 2018 at 3:49 PM Stefan Theil stefan.theil@mixed-mode.de wrote:
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then. Also flushing the receive buffers in the transmit function makes no sense and can be removed.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
Changes for v2: - Use invalidate_dcache_range instead of flush_dcache_range Changes for v3: - Remove unnecessary flushing of all RX buffers in zynq_gem_send Changes for v4: - Invalidate receive buffers after allocating them in zynq_gem_probe
drivers/net/zynq_gem.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..79a22fb1ed 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -570,11 +570,6 @@ static int zynq_gem_send(struct udevice *dev, void *ptr, int len) addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr + size);
addr = (ulong)priv->rxbuffers;addr &= ~(ARCH_DMA_MINALIGN - 1);size = roundup((RX_BUF * PKTSIZE_ALIGN), ARCH_DMA_MINALIGN);flush_dcache_range(addr, addr + size); barrier(); /* Start transmit */@@ -621,6 +616,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
invalidate_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));barrier();return frame_len;}
@@ -705,7 +703,9 @@ static int zynq_gem_probe(struct udevice *dev) if (!priv->rxbuffers) return -ENOMEM;
memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN);
u32 addr = (ulong)priv->rxbuffers;invalidate_dcache_range(addr, addr + roundup(RX_BUF * PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));barrier();
Does this fix anything? I see no need to update this.
/* Align bd_space to MMU_SECTION_SHIFT */ bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);--
Regards, Bin
-----Ursprüngliche Nachricht----- Von: Bin Meng [mailto:bmeng.cn@gmail.com] Gesendet: Montag, 17. Dezember 2018 08:52 An: Stefan Theil Cc: U-Boot Mailing List; Michal Simek Betreff: Re: [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
Hi Stefan,
On Mon, Dec 17, 2018 at 3:49 PM Stefan Theil <stefan.theil@mixed- mode.de> wrote:
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then. Also flushing the receive buffers in the transmit function makes no sense and can be removed.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
Changes for v2: - Use invalidate_dcache_range instead of flush_dcache_range Changes for v3: - Remove unnecessary flushing of all RX buffers in zynq_gem_send Changes for v4: - Invalidate receive buffers after allocating them in zynq_gem_probe
drivers/net/zynq_gem.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..79a22fb1ed 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -570,11 +570,6 @@ static int zynq_gem_send(struct udevice *dev,
void *ptr, int len)
addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr + size);
addr = (ulong)priv->rxbuffers;addr &= ~(ARCH_DMA_MINALIGN - 1);size = roundup((RX_BUF * PKTSIZE_ALIGN), ARCH_DMA_MINALIGN);flush_dcache_range(addr, addr + size); barrier(); /* Start transmit */@@ -621,6 +616,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
invalidate_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN,ARCH_DMA_MINALIGN));
barrier();return frame_len;}
@@ -705,7 +703,9 @@ static int zynq_gem_probe(struct udevice *dev) if (!priv->rxbuffers) return -ENOMEM;
memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN);
u32 addr = (ulong)priv->rxbuffers;invalidate_dcache_range(addr, addr + roundup(RX_BUF *PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
barrier();Does this fix anything? I see no need to update this.
I was just following Michal's suggestion (https://lists.denx.de/pipermail/u-boot/2018-December/351969.html):
Also in probe there should be flush of priv->rxbuffers to make sure that it is initialized properly. (memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN); - this should be useless)
/* Align bd_space to MMU_SECTION_SHIFT */ bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);--
Regards, Bin
Regards, Stefan
Hi Stefan,
On Mon, Dec 17, 2018 at 3:57 PM Stefan Theil Stefan.Theil@mixed-mode.de wrote:
-----Ursprüngliche Nachricht----- Von: Bin Meng [mailto:bmeng.cn@gmail.com] Gesendet: Montag, 17. Dezember 2018 08:52 An: Stefan Theil Cc: U-Boot Mailing List; Michal Simek Betreff: Re: [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
Hi Stefan,
On Mon, Dec 17, 2018 at 3:49 PM Stefan Theil <stefan.theil@mixed- mode.de> wrote:
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then. Also flushing the receive buffers in the transmit function makes no sense and can be removed.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
Changes for v2: - Use invalidate_dcache_range instead of flush_dcache_range Changes for v3: - Remove unnecessary flushing of all RX buffers in zynq_gem_send Changes for v4: - Invalidate receive buffers after allocating them in zynq_gem_probe
drivers/net/zynq_gem.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..79a22fb1ed 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -570,11 +570,6 @@ static int zynq_gem_send(struct udevice *dev,
void *ptr, int len)
addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr + size);
addr = (ulong)priv->rxbuffers;addr &= ~(ARCH_DMA_MINALIGN - 1);size = roundup((RX_BUF * PKTSIZE_ALIGN), ARCH_DMA_MINALIGN);flush_dcache_range(addr, addr + size); barrier(); /* Start transmit */@@ -621,6 +616,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
invalidate_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN,ARCH_DMA_MINALIGN));
barrier();return frame_len;}
@@ -705,7 +703,9 @@ static int zynq_gem_probe(struct udevice *dev) if (!priv->rxbuffers) return -ENOMEM;
memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN);
u32 addr = (ulong)priv->rxbuffers;invalidate_dcache_range(addr, addr + roundup(RX_BUF *PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
barrier();Does this fix anything? I see no need to update this.
I was just following Michal's suggestion (https://lists.denx.de/pipermail/u-boot/2018-December/351969.html):
Also in probe there should be flush of priv->rxbuffers to make sure that it is initialized properly. (memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN); - this should be useless)
Michal was suggesting 'flush', not 'invalidate', to make sure priv->rxbuffers is really initialized to zero by memset().
/* Align bd_space to MMU_SECTION_SHIFT */ bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);--
Regards, Bin
-----Ursprüngliche Nachricht----- Von: Bin Meng [mailto:bmeng.cn@gmail.com] Gesendet: Montag, 17. Dezember 2018 09:05 An: Stefan Theil Cc: U-Boot Mailing List; Michal Simek Betreff: Re: [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
Hi Stefan,
On Mon, Dec 17, 2018 at 3:57 PM Stefan Theil <Stefan.Theil@mixed- mode.de> wrote:
-----Ursprüngliche Nachricht----- Von: Bin Meng [mailto:bmeng.cn@gmail.com] Gesendet: Montag, 17. Dezember 2018 08:52 An: Stefan Theil Cc: U-Boot Mailing List; Michal Simek Betreff: Re: [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
Hi Stefan,
On Mon, Dec 17, 2018 at 3:49 PM Stefan Theil <stefan.theil@mixed- mode.de> wrote:
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then. Also flushing the receive buffers in the transmit function makes no sense and can be removed.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
Changes for v2: - Use invalidate_dcache_range instead of flush_dcache_range Changes for v3: - Remove unnecessary flushing of all RX buffers in zynq_gem_send Changes for v4: - Invalidate receive buffers after allocating them in zynq_gem_probe
drivers/net/zynq_gem.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..79a22fb1ed 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -570,11 +570,6 @@ static int zynq_gem_send(struct udevice *dev,
void *ptr, int len)
addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr + size);
addr = (ulong)priv->rxbuffers;addr &= ~(ARCH_DMA_MINALIGN - 1);size = roundup((RX_BUF * PKTSIZE_ALIGN),ARCH_DMA_MINALIGN);
flush_dcache_range(addr, addr + size); barrier(); /* Start transmit */@@ -621,6 +616,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
invalidate_dcache_range(addr, addr +- roundup(PKTSIZE_ALIGN,
ARCH_DMA_MINALIGN));
barrier();return frame_len;}
@@ -705,7 +703,9 @@ static int zynq_gem_probe(struct udevice *dev) if (!priv->rxbuffers) return -ENOMEM;
memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN);
u32 addr = (ulong)priv->rxbuffers;invalidate_dcache_range(addr, addr + roundup(RX_BUF *PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
barrier();Does this fix anything? I see no need to update this.
I was just following Michal's suggestion (https://lists.denx.de/pipermail/u-
boot/2018-December/351969.html):
Also in probe there should be flush of priv->rxbuffers to make sure that it is initialized properly. (memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN); - this should be useless)
Michal was suggesting 'flush', not 'invalidate', to make sure priv->rxbuffers is really initialized to zero by memset().
Oh you're right. Shouldn't submit patches before my first coffee... I'll fix that.
/* Align bd_space to MMU_SECTION_SHIFT */ bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);--
Regards, Bin
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then. Also flushing the receive buffers in the transmit function makes no sense and can be removed.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
--- Changes for v2: - Use invalidate_dcache_range instead of flush_dcache_range Changes for v3: - Remove unnecessary flushing of all RX buffers in zynq_gem_send Changes for v4: - Invalidate receive buffers after allocating them in zynq_gem_probe Changes for v5: - Clear and flush receive buffers in zynq_gem_probe instead of invalidating the cache --- drivers/net/zynq_gem.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..3bd0093b7a 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -570,11 +570,6 @@ static int zynq_gem_send(struct udevice *dev, void *ptr, int len) addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr + size); - - addr = (ulong)priv->rxbuffers; - addr &= ~(ARCH_DMA_MINALIGN - 1); - size = roundup((RX_BUF * PKTSIZE_ALIGN), ARCH_DMA_MINALIGN); - flush_dcache_range(addr, addr + size); barrier();
/* Start transmit */ @@ -621,6 +616,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
+ invalidate_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN, ARCH_DMA_MINALIGN)); + barrier(); + return frame_len; }
@@ -706,6 +704,9 @@ static int zynq_gem_probe(struct udevice *dev) return -ENOMEM;
memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN); + u32 addr = (ulong)priv->rxbuffers; + flush_dcache_range(addr, addr + roundup(RX_BUF * PKTSIZE_ALIGN, ARCH_DMA_MINALIGN)); + barrier();
/* Align bd_space to MMU_SECTION_SHIFT */ bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
On Mon, Dec 17, 2018 at 4:12 PM Stefan Theil stefan.theil@mixed-mode.de wrote:
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then. Also flushing the receive buffers in the transmit function makes no sense and can be removed.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
Changes for v2: - Use invalidate_dcache_range instead of flush_dcache_range Changes for v3: - Remove unnecessary flushing of all RX buffers in zynq_gem_send Changes for v4: - Invalidate receive buffers after allocating them in zynq_gem_probe Changes for v5: - Clear and flush receive buffers in zynq_gem_probe instead of invalidating the cache
drivers/net/zynq_gem.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On 17. 12. 18 9:12, Stefan Theil wrote:
The cache was only flushed before *transmitting* packets, but not when receiving them, leading to an issue where new packets were handed to the receive handler with old contents in cache. This only happens when a lot of packets are received without sending packages every now and then. Also flushing the receive buffers in the transmit function makes no sense and can be removed.
Signed-off-by: Stefan Theil stefan.theil@mixed-mode.de
Changes for v2:
- Use invalidate_dcache_range instead of flush_dcache_range
Changes for v3:
- Remove unnecessary flushing of all RX buffers in zynq_gem_send
Changes for v4:
- Invalidate receive buffers after allocating them in zynq_gem_probe
Changes for v5:
- Clear and flush receive buffers in zynq_gem_probe instead of invalidating the cache
drivers/net/zynq_gem.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 9bd79b198a..3bd0093b7a 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -570,11 +570,6 @@ static int zynq_gem_send(struct udevice *dev, void *ptr, int len) addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr + size);
addr = (ulong)priv->rxbuffers;
addr &= ~(ARCH_DMA_MINALIGN - 1);
size = roundup((RX_BUF * PKTSIZE_ALIGN), ARCH_DMA_MINALIGN);
flush_dcache_range(addr, addr + size); barrier();
/* Start transmit */
@@ -621,6 +616,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
- invalidate_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
- barrier();
- return frame_len;
}
@@ -706,6 +704,9 @@ static int zynq_gem_probe(struct udevice *dev) return -ENOMEM;
memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN);
u32 addr = (ulong)priv->rxbuffers;
flush_dcache_range(addr, addr + roundup(RX_BUF * PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
barrier();
/* Align bd_space to MMU_SECTION_SHIFT */ bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
Applied. Michal
participants (4)
-
Bin Meng -
Michal Simek -
Stefan Theil -
Stefan Theil