Update cache helpers #1606

jenswi-linaro · 2017-06-15T13:57:40Z

No description provided.

jenswi-linaro · 2017-06-15T18:08:20Z

Travis error is due to checkpatch warnings, fixing those warning would make future syncs with the ARM-TF cache helpers a bit more difficult.

etienne-lms

My R-b for the 3 first commits (few minor comments).
For the 4th "core: update cache helpers": 2 comments but my R-b would already apply to the sequences and core registers/instruction set. Cross-checked with arm docs and a-tf sources. very nice integration, thanks.

etienne-lms · 2017-06-15T19:36:58Z

core/arch/arm/include/arm32_macros.S

@@ -120,11 +115,6 @@
 	mcr	p15, 0, r0, c7, c1, 0
 	.endm

-	.macro write_bpiall


minor about sorting, write_bpiall should remain here (with instr. fix:).

I'm sorry, I don't understand. Please explain.

my mistake :( discard my comment, I just understood the register sorting.

etienne-lms · 2017-06-15T21:42:07Z

core/arch/arm/kernel/generic_entry_a32.S

@@ -260,7 +260,7 @@ END_FUNC reset
 		assert_flat_mapped_range (\vbase), (\line)
 		ldr	r0, =(\vbase)
 		ldr	r1, =(\vend)
-		bl	arm_cl1_d_cleanbyva
+		bl	dcache_clean_range


add sub r1, r1, r0

etienne-lms · 2017-06-15T21:48:17Z

core/arch/arm/kernel/generic_entry_a32.S

@@ -506,7 +506,8 @@ UNWIND(	.cantunwind)

 #if defined (CFG_BOOT_SECONDARY_REQUEST)
 	/* if L1 is not invalidated before, do it here */
-	bl	arm_cl1_d_invbysetway
+	mov	r0, #DCACHE_OP_INV
+	bl	dcache_op_all


this should invalidate only the inner (louis), no ?

Good question, I don't know. On the other hand this is done once during boot so it's not that much time that can be saved by doing it only to louis.

invalidating all data cache could be harmful to other running cores, no?

Good point, I'll change to dcache_op_louis(). @yanyan-wrs, do you agree?

yes. maybe we should ask some imx maintainers to test this.

%grep -l CFG_BOOT_SECONDARY_REQUEST **/*.mk core/arch/arm/plat-imx/conf.mk core/arch/arm/plat-ls/conf.mk core/arch/arm/plat-zynq7k/conf.mk mk/config.mk

So any of plat-imx, plat-ls or plat-zynq7k should be useful.
Any volunteers/comments, @yanyan-wrs , @sorenb-xlnx , @b49020 ?

etienne-lms · 2017-06-15T22:08:38Z

core/arch/arm/include/arm32_macros.S

+	.endm
+
+	.macro write_dcimvac reg
+	/* Data cache clean by MVA */


typo: s/clean/invalidate/

jenswi-linaro · 2017-06-16T07:17:10Z

Addressing review comments (except the one about not moving write_bpiall).

jenswi-linaro · 2017-06-16T09:15:10Z

update

etienne-lms · 2017-06-16T12:07:24Z

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>

jenswi-linaro · 2017-06-19T19:56:33Z

I'll assume that everyone is happy with this and squash and rebase in 2 days.

etienne-lms · 2017-06-20T08:16:47Z

core/arch/arm/kernel/generic_entry_a32.S

@@ -504,7 +508,8 @@ UNWIND(	.cantunwind)

 #if defined (CFG_BOOT_SECONDARY_REQUEST)
 	/* if L1 is not invalidated before, do it here */
-	bl	arm_cl1_d_invbysetway
+	mov	r0, #DCACHE_OP_INV
+	bl	dcache_op_louis


Sorry, late comment after reading back this.
Actually, i even think this should invalidate only the L1 (dcache_op_level1), not the L2 (in case it is in the inner) that might be used by other running cores.

Yes, it makes more sense.

etienne-lms · 2017-06-20T12:59:05Z

Just a minor comment about sorting in core/arch/arm/include/arm32_macros.S.
The ARM documentation usually lists (sorts) the CP15 registers by indices (N,M,O,P) in "cN, M, cO, P", instead of sorting by (M, N, O, P) as proposed in 2nd commit.
I think the a32_macros.S just better use the ARM docs sort scheme.

jenswi-linaro · 2017-06-20T13:14:37Z

I'm not proposing any sorting order in the 2nd commit, I'm just correcting some deviations from the comment about the sorting. The main reason for a sorting order is to avoid the problem with conflicts when adding functions at the end. Other than that I don't care, but I'm not too happy about changing the sorting order as it doubtless will cause conflicts when rebasing these patches.

When you write "cN, M, cO, P" what is what? I'm asking since the instructions are written as p15, A, rX, cB, cC, D.

etienne-lms · 2017-06-20T13:33:21Z

When you write "cN, M, cO, P" what is what? I'm asking since the instructions are written as p15, A, rX, cB, cC, D.

I meant registers targeted by instructions based on "p15, A, rX, cB, cC, D" are sorted by (B, A, C, D) in the literature. (up to my experience at least)

jenswi-linaro · 2017-06-21T12:22:06Z

@etienne-lms are you OK with the current state or do we need more changes?

etienne-lms · 2017-06-21T17:49:29Z

@jenswi-linaro: yes, current state of the PR is fine to me (Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>). late reply, connection issues from home :( this afternoon

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

Sorts macros and fixes the macro write_bpial Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

Updates AArch64 and ARMv7 cache helpers from lib/aarch32/cache_helpers.S and lib/aarch64/cache_helpers.S in ARM-TF, https://github.com/ARM-software/arm-trusted-firmware/tree/2bd26faf62411c75111fea4b23c542865383b068 The imported routines only covers the inner cache. Already present ARMv7 cache routines are replaced by the new equivalent routines. The AArch64 routines are updated with the resent changes in ARM-TF. The imported files are modified to better fit into OP-TEE, some functions and defines are renamed. Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Tested-by: Jens Wiklander <jens.wiklander@linaro.org> (Hikey AArch{32,64} pager) Tested-by: Jens Wiklander <jens.wiklander@linaro.org> (Juno AArch{32,64} pager) Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

jenswi-linaro · 2017-06-21T20:09:11Z

Tags applied.

etienne-lms reviewed Jun 15, 2017

View reviewed changes

etienne-lms mentioned this pull request Jun 19, 2017

Add i.MX PSCI CPU off and update i.MX6UL #1577

Merged

etienne-lms reviewed Jun 20, 2017

View reviewed changes

jenswi-linaro added 4 commits June 21, 2017 21:22

core: arm.h: add cache related defines

52f2c65

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

core: arm32_macros.S: sort macros

03afb20

Sorts macros and fixes the macro write_bpial Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

core: arm32_macros.S: add cache related macros

d3880cd

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

jenswi-linaro force-pushed the cache_helpers branch from dbdc78f to b3177a3 Compare June 21, 2017 20:08

jforissier merged commit 13f187f into OP-TEE:master Jun 21, 2017

jenswi-linaro deleted the cache_helpers branch June 22, 2017 06:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update cache helpers #1606

Update cache helpers #1606

jenswi-linaro commented Jun 15, 2017

jenswi-linaro commented Jun 15, 2017

etienne-lms left a comment

etienne-lms Jun 15, 2017

jenswi-linaro Jun 16, 2017

etienne-lms Jun 16, 2017

etienne-lms Jun 15, 2017

etienne-lms Jun 15, 2017

jenswi-linaro Jun 16, 2017

etienne-lms Jun 16, 2017

jenswi-linaro Jun 16, 2017

etienne-lms Jun 16, 2017

jenswi-linaro Jun 16, 2017

etienne-lms Jun 15, 2017

jenswi-linaro commented Jun 16, 2017

jenswi-linaro commented Jun 16, 2017

etienne-lms commented Jun 16, 2017

jenswi-linaro commented Jun 19, 2017

etienne-lms Jun 20, 2017

jenswi-linaro Jun 20, 2017

etienne-lms commented Jun 20, 2017

jenswi-linaro commented Jun 20, 2017

etienne-lms commented Jun 20, 2017

jenswi-linaro commented Jun 21, 2017

etienne-lms commented Jun 21, 2017

jenswi-linaro commented Jun 21, 2017

Update cache helpers #1606

Update cache helpers #1606

Conversation

jenswi-linaro commented Jun 15, 2017

jenswi-linaro commented Jun 15, 2017

etienne-lms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenswi-linaro commented Jun 16, 2017

jenswi-linaro commented Jun 16, 2017

etienne-lms commented Jun 16, 2017

jenswi-linaro commented Jun 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etienne-lms commented Jun 20, 2017

jenswi-linaro commented Jun 20, 2017

etienne-lms commented Jun 20, 2017

jenswi-linaro commented Jun 21, 2017

etienne-lms commented Jun 21, 2017

jenswi-linaro commented Jun 21, 2017