On 01/03/18 23:27, Stefano Stabellini wrote:
See the corresponding Linux commit as reference:

   commit f91e2c3bd427239c198351f44814dd39db91afe0
   Author: Catalin Marinas <catalin.mari...@arm.com>
   Date:   Tue Dec 7 16:52:04 2010 +0100

       ARM: 6527/1: Use CTR instead of CCSIDR for the D-cache line size on ARMv7

       The current implementation of the dcache_line_size macro reads the L1
       cache size from the CCSIDR register. This, however, is not guaranteed to
       be the smallest cache line in the cache hierarchy. The patch changes to
       the macro to use the more architecturally correct CTR register.

       Reported-by: Kevin Sapp <ks...@quicinc.com>
       Signed-off-by: Catalin Marinas <catalin.mari...@arm.com>
       Signed-off-by: Russell King <rmk+ker...@arm.linux.org.uk>

Suggested-by: Julien Grall <julien.gr...@arm.com>
Signed-off-by: Stefano Stabellini <sstabell...@kernel.org>


This patch depends on "unsafe big.LITTLE support".

I still really don't think this should depend on "unsafe big.LITTLE support". We want to backport this patch but I am still unconvinced that this is the case of the big.LITTLE one. So can you please reshuffle the patches?

Previously, we discussed the possibility of reading the cacheline size
when needed from the register, instead of reading it from a variable,
but going through the code it doesn't seem like a worthwhile

Well there are a couple of reasons I wanted this to avoid the a variable:
1) Potentially reading a system register + few instructions is faster than a memory access 2) The name of the variable leads to confusing. It is named cacheline_bytes but stores the minimum cacheline size of for the data cache.

1) is arguable and I don't much care whether it is done. However, I really want to avoid a wrong variable name that could lead to more misuse. So we should at least rename read_cacheline_size() and cacheline_bytes.


  xen/include/asm-arm/cpregs.h |  2 ++
  xen/include/asm-arm/page.h   | 11 +++++------
  2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
index 9e13848..8db65d5 100644
--- a/xen/include/asm-arm/cpregs.h
+++ b/xen/include/asm-arm/cpregs.h
@@ -106,6 +106,7 @@
/* CP15 CR0: CPUID and Cache Type Registers */
  #define MIDR            p15,0,c0,c0,0   /* Main ID Register */
+#define CTR             p15,0,c0,c0,1   /* Cache Type Register */
  #define MPIDR           p15,0,c0,c0,5   /* Multiprocessor Affinity Register */
  #define ID_PFR0         p15,0,c0,c1,0   /* Processor Feature Register 0 */
  #define ID_PFR1         p15,0,c0,c1,1   /* Processor Feature Register 1 */
@@ -303,6 +304,7 @@
  #define CPACR_EL1               CPACR
  #define CPTR_EL2                HCPTR
  #define CSSELR_EL1              CSSELR
+#define CTR_EL0                 CTR
  #define DACR32_EL2              DACR
  #define ESR_EL1                 DFSR
  #define ESR_EL2                 HSR
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 9fbf232..4c81878 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -140,14 +140,13 @@ extern size_t cacheline_bytes;
static inline size_t read_cacheline_size(void)
-    uint32_t ccsid;
+    uint32_t ctr;
- /* Read the cache size ID register for the level-0 data cache */
-    ccsid = READ_SYSREG32(CCSIDR_EL1);
+    /* Read CTR */
+    ctr = READ_SYSREG32(CTR_EL0);
- /* Low 3 bits are log2(cacheline size in words) - 2. */
-    return (size_t) (1U << (4 + (ccsid & 0x7)));
+    /* Bits 16-19 are the log2 number of words in the cacheline. */
+    return (size_t) (4 << ((ctr >> 16) & 0xf));
/* Functions for flushing medium-sized areas.

Julien Grall

Xen-devel mailing list

Reply via email to