diff mbox series

[kvm-unit-tests,RFC,12/19] arm/arm64: assembler.h: Replace size with end address for dcache_by_line_op

Message ID 20220809091558.14379-13-alexandru.elisei@arm.com (mailing list archive)
State New, archived
Headers show
Series arm/arm64: Rework cache maintenance at boot | expand

Commit Message

Alexandru Elisei Aug. 9, 2022, 9:15 a.m. UTC
Commit b5f659be4775 ("arm/arm64: Remove dcache_line_size global
variable") moved the dcache_by_line_op macro to assembler.h and changed
it to take the size of the regions instead of the end address as
parameter. This was done to keep the file in sync with the upstream
Linux kernel implementation at the time.

But in both places where the macro is used, the code has the start and
end address of the region, and it has to compute the size to pass it to
dcache_by_line_op. Then the macro itsef computes the end by adding size
to start.

Get rid of this massaging of parameters and change the macro to the end
address as parameter directly.

Besides slightly simplyfing the code by remove two unneeded arithmetic
operations, this makes the macro compatible with the current upstream
version of Linux (which was similarly changed to take the end address in
commit 163d3f80695e ("arm64: dcache_by_line_op to take end parameter
instead of size")), which will allow us to reuse (part of) the Linux C
wrappers over the assembly macro.

The change has been tested with the same snippet of code used to test
commit 410b3bf09e76 ("arm/arm64: Perform dcache clean + invalidate after
turning MMU off").

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/cstart.S              |  1 -
 arm/cstart64.S            |  1 -
 lib/arm/asm/assembler.h   | 11 +++++------
 lib/arm64/asm/assembler.h | 11 +++++------
 4 files changed, 10 insertions(+), 14 deletions(-)

Comments

Nikos Nikoleris Aug. 9, 2022, 1:01 p.m. UTC | #1
On 09/08/2022 10:15, Alexandru Elisei wrote:
> Commit b5f659be4775 ("arm/arm64: Remove dcache_line_size global
> variable") moved the dcache_by_line_op macro to assembler.h and changed
> it to take the size of the regions instead of the end address as
> parameter. This was done to keep the file in sync with the upstream
> Linux kernel implementation at the time.
> 
> But in both places where the macro is used, the code has the start and
> end address of the region, and it has to compute the size to pass it to
> dcache_by_line_op. Then the macro itsef computes the end by adding size
> to start.
> 
> Get rid of this massaging of parameters and change the macro to the end
> address as parameter directly.
> 
> Besides slightly simplyfing the code by remove two unneeded arithmetic
> operations, this makes the macro compatible with the current upstream
> version of Linux (which was similarly changed to take the end address in
> commit 163d3f80695e ("arm64: dcache_by_line_op to take end parameter
> instead of size")), which will allow us to reuse (part of) the Linux C
> wrappers over the assembly macro.
> 
> The change has been tested with the same snippet of code used to test
> commit 410b3bf09e76 ("arm/arm64: Perform dcache clean + invalidate after
> turning MMU off").
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>

Thanks,

Nikos

> ---
>   arm/cstart.S              |  1 -
>   arm/cstart64.S            |  1 -
>   lib/arm/asm/assembler.h   | 11 +++++------
>   lib/arm64/asm/assembler.h | 11 +++++------
>   4 files changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/arm/cstart.S b/arm/cstart.S
> index 39e70f40986a..096a77c454f4 100644
> --- a/arm/cstart.S
> +++ b/arm/cstart.S
> @@ -226,7 +226,6 @@ asm_mmu_disable:
>   	ldr	r0, [r0]
>   	ldr	r1, =__phys_end
>   	ldr	r1, [r1]
> -	sub	r1, r1, r0
>   	dcache_by_line_op dccimvac, sy, r0, r1, r2, r3
>   
>   	mov     pc, lr
> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index 54773676d1d5..7cc90a9fa13f 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -258,7 +258,6 @@ asm_mmu_disable:
>   	ldr	x0, [x0, :lo12:__phys_offset]
>   	adrp	x1, __phys_end
>   	ldr	x1, [x1, :lo12:__phys_end]
> -	sub	x1, x1, x0
>   	dcache_by_line_op civac, sy, x0, x1, x2, x3
>   
>   	ret
> diff --git a/lib/arm/asm/assembler.h b/lib/arm/asm/assembler.h
> index 4200252dd14d..db5f0f55027c 100644
> --- a/lib/arm/asm/assembler.h
> +++ b/lib/arm/asm/assembler.h
> @@ -25,17 +25,16 @@
>   
>   /*
>    * Macro to perform a data cache maintenance for the interval
> - * [addr, addr + size).
> + * [addr, end).
>    *
>    * 	op:		operation to execute
>    * 	domain		domain used in the dsb instruction
>    * 	addr:		starting virtual address of the region
> - * 	size:		size of the region
> - * 	Corrupts:	addr, size, tmp1, tmp2
> + * 	end:		the end of the region (non-inclusive)
> + * 	Corrupts:	addr, tmp1, tmp2
>    */
> -	.macro dcache_by_line_op op, domain, addr, size, tmp1, tmp2
> +	.macro dcache_by_line_op op, domain, addr, end, tmp1, tmp2
>   	dcache_line_size \tmp1, \tmp2
> -	add	\size, \addr, \size
>   	sub	\tmp2, \tmp1, #1
>   	bic	\addr, \addr, \tmp2
>   9998:
> @@ -45,7 +44,7 @@
>   	.err
>   	.endif
>   	add	\addr, \addr, \tmp1
> -	cmp	\addr, \size
> +	cmp	\addr, \end
>   	blo	9998b
>   	dsb	\domain
>   	.endm
> diff --git a/lib/arm64/asm/assembler.h b/lib/arm64/asm/assembler.h
> index aa8c65a2bb4a..1e09d65af4a7 100644
> --- a/lib/arm64/asm/assembler.h
> +++ b/lib/arm64/asm/assembler.h
> @@ -28,25 +28,24 @@
>   
>   /*
>    * Macro to perform a data cache maintenance for the interval
> - * [addr, addr + size). Use the raw value for the dcache line size because
> + * [addr, end). Use the raw value for the dcache line size because
>    * kvm-unit-tests has no concept of scheduling.
>    *
>    * 	op:		operation passed to dc instruction
>    * 	domain:		domain used in dsb instruction
>    * 	addr:		starting virtual address of the region
> - * 	size:		size of the region
> - * 	Corrupts:	addr, size, tmp1, tmp2
> + * 	end:		the end of the region (non-inclusive)
> + * 	Corrupts:	addr, tmp1, tmp2
>    */
>   
> -	.macro dcache_by_line_op op, domain, addr, size, tmp1, tmp2
> +	.macro dcache_by_line_op op, domain, addr, end, tmp1, tmp2
>   	raw_dcache_line_size \tmp1, \tmp2
> -	add	\size, \addr, \size
>   	sub	\tmp2, \tmp1, #1
>   	bic	\addr, \addr, \tmp2
>   9998:
>   	dc	\op, \addr
>   	add	\addr, \addr, \tmp1
> -	cmp	\addr, \size
> +	cmp	\addr, \end
>   	b.lo	9998b
>   	dsb	\domain
>   	.endm
Andrew Jones Sept. 20, 2022, 9:37 a.m. UTC | #2
On Tue, Aug 09, 2022 at 10:15:51AM +0100, Alexandru Elisei wrote:
> Commit b5f659be4775 ("arm/arm64: Remove dcache_line_size global
> variable") moved the dcache_by_line_op macro to assembler.h and changed
> it to take the size of the regions instead of the end address as
> parameter. This was done to keep the file in sync with the upstream
> Linux kernel implementation at the time.
> 
> But in both places where the macro is used, the code has the start and
> end address of the region, and it has to compute the size to pass it to
> dcache_by_line_op. Then the macro itsef computes the end by adding size
> to start.
> 
> Get rid of this massaging of parameters and change the macro to the end
> address as parameter directly.
> 
> Besides slightly simplyfing the code by remove two unneeded arithmetic
> operations, this makes the macro compatible with the current upstream
> version of Linux (which was similarly changed to take the end address in
> commit 163d3f80695e ("arm64: dcache_by_line_op to take end parameter
> instead of size")), which will allow us to reuse (part of) the Linux C
> wrappers over the assembly macro.
> 
> The change has been tested with the same snippet of code used to test
> commit 410b3bf09e76 ("arm/arm64: Perform dcache clean + invalidate after
> turning MMU off").
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/cstart.S              |  1 -
>  arm/cstart64.S            |  1 -
>  lib/arm/asm/assembler.h   | 11 +++++------
>  lib/arm64/asm/assembler.h | 11 +++++------
>  4 files changed, 10 insertions(+), 14 deletions(-)
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
diff mbox series

Patch

diff --git a/arm/cstart.S b/arm/cstart.S
index 39e70f40986a..096a77c454f4 100644
--- a/arm/cstart.S
+++ b/arm/cstart.S
@@ -226,7 +226,6 @@  asm_mmu_disable:
 	ldr	r0, [r0]
 	ldr	r1, =__phys_end
 	ldr	r1, [r1]
-	sub	r1, r1, r0
 	dcache_by_line_op dccimvac, sy, r0, r1, r2, r3
 
 	mov     pc, lr
diff --git a/arm/cstart64.S b/arm/cstart64.S
index 54773676d1d5..7cc90a9fa13f 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -258,7 +258,6 @@  asm_mmu_disable:
 	ldr	x0, [x0, :lo12:__phys_offset]
 	adrp	x1, __phys_end
 	ldr	x1, [x1, :lo12:__phys_end]
-	sub	x1, x1, x0
 	dcache_by_line_op civac, sy, x0, x1, x2, x3
 
 	ret
diff --git a/lib/arm/asm/assembler.h b/lib/arm/asm/assembler.h
index 4200252dd14d..db5f0f55027c 100644
--- a/lib/arm/asm/assembler.h
+++ b/lib/arm/asm/assembler.h
@@ -25,17 +25,16 @@ 
 
 /*
  * Macro to perform a data cache maintenance for the interval
- * [addr, addr + size).
+ * [addr, end).
  *
  * 	op:		operation to execute
  * 	domain		domain used in the dsb instruction
  * 	addr:		starting virtual address of the region
- * 	size:		size of the region
- * 	Corrupts:	addr, size, tmp1, tmp2
+ * 	end:		the end of the region (non-inclusive)
+ * 	Corrupts:	addr, tmp1, tmp2
  */
-	.macro dcache_by_line_op op, domain, addr, size, tmp1, tmp2
+	.macro dcache_by_line_op op, domain, addr, end, tmp1, tmp2
 	dcache_line_size \tmp1, \tmp2
-	add	\size, \addr, \size
 	sub	\tmp2, \tmp1, #1
 	bic	\addr, \addr, \tmp2
 9998:
@@ -45,7 +44,7 @@ 
 	.err
 	.endif
 	add	\addr, \addr, \tmp1
-	cmp	\addr, \size
+	cmp	\addr, \end
 	blo	9998b
 	dsb	\domain
 	.endm
diff --git a/lib/arm64/asm/assembler.h b/lib/arm64/asm/assembler.h
index aa8c65a2bb4a..1e09d65af4a7 100644
--- a/lib/arm64/asm/assembler.h
+++ b/lib/arm64/asm/assembler.h
@@ -28,25 +28,24 @@ 
 
 /*
  * Macro to perform a data cache maintenance for the interval
- * [addr, addr + size). Use the raw value for the dcache line size because
+ * [addr, end). Use the raw value for the dcache line size because
  * kvm-unit-tests has no concept of scheduling.
  *
  * 	op:		operation passed to dc instruction
  * 	domain:		domain used in dsb instruction
  * 	addr:		starting virtual address of the region
- * 	size:		size of the region
- * 	Corrupts:	addr, size, tmp1, tmp2
+ * 	end:		the end of the region (non-inclusive)
+ * 	Corrupts:	addr, tmp1, tmp2
  */
 
-	.macro dcache_by_line_op op, domain, addr, size, tmp1, tmp2
+	.macro dcache_by_line_op op, domain, addr, end, tmp1, tmp2
 	raw_dcache_line_size \tmp1, \tmp2
-	add	\size, \addr, \size
 	sub	\tmp2, \tmp1, #1
 	bic	\addr, \addr, \tmp2
 9998:
 	dc	\op, \addr
 	add	\addr, \addr, \tmp1
-	cmp	\addr, \size
+	cmp	\addr, \end
 	b.lo	9998b
 	dsb	\domain
 	.endm