[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xen/arm: Read the cacheline size from CTR register
On 02/03/2018 18:33, Stefano Stabellini wrote: On Fri, 2 Mar 2018, Stefano Stabellini wrote:On Fri, 2 Mar 2018, Julien Grall wrote:Hi, On 01/03/18 23:27, Stefano Stabellini wrote:See the corresponding Linux commit as reference: commit f91e2c3bd427239c198351f44814dd39db91afe0 Author: Catalin Marinas <catalin.marinas@xxxxxxx> 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 <ksapp@xxxxxxxxxxx> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx> Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx> Suggested-by: Julien Grall <julien.grall@xxxxxxx> Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> --- 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?I'll move it earlier. For simplicity, I'll make it the first patch of "unsafe big.LITTLE support", althought I understand that they are separate fixes.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 optimization.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.Sure, I can rename. Would min_cacheline_bytes and read_min_cacheline_bytes() be clearer? Otherwise, please suggest an alternative naming scheme.Or min_dcache_line_bytes ? I would reduce to dcache_line_bytes to avoid lengthy name.That would go inline with the macro dcache_line_size we currently have in arm64. What matters is the 'd' for data cache. To avoid confusion which 'i' for instruction cache that we may need in the future. Anyway, I am happy with both. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |