Hi,
On 20/02/2018 21:03, Stefano Stabellini wrote:
On Tue, 20 Feb 2018, Julien Grall wrote:
On 19/02/18 21:58, Stefano Stabellini wrote:
+ mrc CP32(r6, CSSELR_EL1)
The size of the cache is read using CSSIDR_EL1. But it looks like the way we
get the cache line size in Xen is fragile.
We are retrieving the cache line size of Level 1 and assume this will be valid
for all the other caches. Indeed cache maintenance ops may propagate to other
caches depending the target (Point of Coherency vs Point of Unification).
Looking at the ARM ARM "Cache hierarchy abstraction for address-based
operations" (D3-2061 DDI 0487C.a), CTR_EL0/CTR will holds the minimum line
lenght values for the data caches. The value will be the most efficient
address stride to use to apply a sequence of VA-based maintenance instructions
to a range of VAs.
So it would be best and safer for Xen to use CTR/CTLR_EL0.DminLine.
This is insightful, thank you. Given that this patch is a backport
candidate, I would prefer to retain the same behavior we had before in
setup_cache. I can write a separate patch on top of this to make the
change to use CTR/CTLR_EL0.DminLine. That way, we can make a separate
decision on each of them on whether we want to backport them (and
potentially revert them) or not. In other words: this patch as-is is
suboptimal but is of very little risk. Making changes to the way we
determine the cacheline size improves the patch but significantly
increases the risk factor associated with it.
Does it make sense?
By this patch you mean big.LITTLE? If so, then I don't consider it as a
potential backport. big.LITTLE has never been supported on Xen and hence
should be considered as a new feature. What is backportable is the patch
#1 that forbid big.LITTLE.
Regarding the cache line size, I didn't suggest the change because it is
more efficient. I suggested the patch because the current code to find
the cache line size is wrong. Imagine there is a cache in the hierarchy
that has a smaller cache line than your L1 cache. Then you would not
clean/invalidate correctly that cache.
+ and r6, r6, #0x7
+ add r6, r6, #4
+ mov r7, #1
+ lsl r6, r7, r6
mov r7, r3
1: mcr CP32(r7, DCCMVAC)
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index fa0ef70..edea300 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -631,8 +631,14 @@ ENTRY(relocate_xen)
dsb sy /* So the CPU issues all writes to the range */
mov x9, x3
- ldr x10, =cacheline_bytes /* x10 := step */
- ldr x10, [x10]
+
+ mov x10, #0
+ msr CSSELR_EL1, x10
+ mrs x10, CSSELR_EL1
+ and x10, x10, #0x7
+ add x10, x10, #4
+ mov x11, #1
+ lsl x10, x11, x10
Please use dcache_line_size macro (see cache.S).
Similarly, I would prefer to retain the same old behavior here, and
fix it/improve it in a separate patch.
See above, you got the wrong end of the stick about the cache line size.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel