[PATCH] ARM64: let 'end' point after the range in cache functions

[PATCH] ARM64: let 'end' point after the range in cache functions

[Enrico Scholz]

From: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>

v8_flush_dcache_range() and v8_inv_dcache_range() are implemented
under the assumption that their 'end' parameter points *after* the
range.

Fix callers to use it in this way.

This fixes e.g. spurious corruptions in the last octet when sending
129 bytes over ethernet.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 arch/arm/cpu/dma_64.c | 2 +-
 arch/arm/cpu/mmu_64.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/dma_64.c b/arch/arm/cpu/dma_64.c
index 74d7167860c2..b50572f5e601 100644
--- a/arch/arm/cpu/dma_64.c
+++ b/arch/arm/cpu/dma_64.c
@@ -6,7 +6,7 @@ void arch_sync_dma_for_device(void *vaddr, size_t size,
                               enum dma_data_direction dir)
 {
 	unsigned long start = (unsigned long)vaddr;
-	unsigned long end = start + size - 1;
+	unsigned long end = start + size;
 
 	if (dir == DMA_FROM_DEVICE)
 		v8_inv_dcache_range(start, end);
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index 12cd644de0c7..b48e4732b86d 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -282,7 +282,7 @@ void mmu_disable(void)
 void dma_inv_range(void *ptr, size_t size)
 {
 	unsigned long start = (unsigned long)ptr;
-	unsigned long end = start + size - 1;
+	unsigned long end = start + size;
 
 	v8_inv_dcache_range(start, end);
 }
@@ -290,7 +290,7 @@ void dma_inv_range(void *ptr, size_t size)
 void dma_flush_range(void *ptr, size_t size)
 {
 	unsigned long start = (unsigned long)ptr;
-	unsigned long end = start + size - 1;
+	unsigned long end = start + size;
 
 	v8_flush_dcache_range(start, end);
 }
-- 
2.44.0

Re: [PATCH] ARM64: let 'end' point after the range in cache functions

[Sascha Hauer]

On Fri, 12 Apr 2024 18:28:35 +0200, Enrico Scholz wrote:
> v8_flush_dcache_range() and v8_inv_dcache_range() are implemented
> under the assumption that their 'end' parameter points *after* the
> range.
> 
> Fix callers to use it in this way.
> 
> This fixes e.g. spurious corruptions in the last octet when sending
> 129 bytes over ethernet.
> 
> [...]

Applied, thanks!

[1/1] ARM64: let 'end' point after the range in cache functions
      https://git.pengutronix.de/cgit/barebox/commit/?id=65ef5d885263 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH] ARM64: let 'end' point after the range in cache functions

[Sascha Hauer]

Hi Enrico,

On Fri, Apr 12, 2024 at 06:28:35PM +0200, Enrico Scholz wrote:
> From: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> 
> v8_flush_dcache_range() and v8_inv_dcache_range() are implemented
> under the assumption that their 'end' parameter points *after* the
> range.
> 
> Fix callers to use it in this way.
> 
> This fixes e.g. spurious corruptions in the last octet when sending
> 129 bytes over ethernet.

So 129 bytes are sent from barebox, right? Which network driver driver
is involved on the barebox side here? How did you force sending excatly
129 bytes?

I am asking because I want to look if there are other bugs invlolved
here.

Sascha

> 
> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> ---
>  arch/arm/cpu/dma_64.c | 2 +-
>  arch/arm/cpu/mmu_64.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/cpu/dma_64.c b/arch/arm/cpu/dma_64.c
> index 74d7167860c2..b50572f5e601 100644
> --- a/arch/arm/cpu/dma_64.c
> +++ b/arch/arm/cpu/dma_64.c
> @@ -6,7 +6,7 @@ void arch_sync_dma_for_device(void *vaddr, size_t size,
>                                enum dma_data_direction dir)
>  {
>  	unsigned long start = (unsigned long)vaddr;
> -	unsigned long end = start + size - 1;
> +	unsigned long end = start + size;
>  
>  	if (dir == DMA_FROM_DEVICE)
>  		v8_inv_dcache_range(start, end);
> diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
> index 12cd644de0c7..b48e4732b86d 100644
> --- a/arch/arm/cpu/mmu_64.c
> +++ b/arch/arm/cpu/mmu_64.c
> @@ -282,7 +282,7 @@ void mmu_disable(void)
>  void dma_inv_range(void *ptr, size_t size)
>  {
>  	unsigned long start = (unsigned long)ptr;
> -	unsigned long end = start + size - 1;
> +	unsigned long end = start + size;
>  
>  	v8_inv_dcache_range(start, end);
>  }
> @@ -290,7 +290,7 @@ void dma_inv_range(void *ptr, size_t size)
>  void dma_flush_range(void *ptr, size_t size)
>  {
>  	unsigned long start = (unsigned long)ptr;
> -	unsigned long end = start + size - 1;
> +	unsigned long end = start + size;
>  
>  	v8_flush_dcache_range(start, end);
>  }
> -- 
> 2.44.0
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] ARM64: let 'end' point after the range in cache functions

[Enrico Scholz]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> So 129 bytes are sent from barebox, right? Which network driver driver
> is involved on the barebox side here? How did you force sending excatly
> 129 bytes?

drivers/net/bcmgenet.c;  I made a

diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
index 9e0bacb31adf..988324cd22d4 100644
--- a/drivers/net/bcmgenet.c
+++ b/drivers/net/bcmgenet.c
@@ -272,6 +272,10 @@ static int bcmgenet_gmac_eth_send(struct eth_device *edev, void *packet, int len
 	u32 tries = 100;
 	dma_addr_t dma;
 
+	if (length == 129)
+		print_hex_dump(KERN_INFO, "D ", DUMP_PREFIX_OFFSET,
+			       16, 4, packet + 125, 4, 1);
+
 	prod_index = readl(priv->mac_reg + TDMA_PROD_INDEX);
 
 	dma = dma_map_single(priv->dev, packet, length, DMA_TO_DEVICE);


there to verify the input data and checked with tcpdump on the other end
(which differed in around 70% of the cases in the last byte).

Packets with arbitrary length can be constructed easily by custom tftp
filenames.



Enrico