Messages in this thread Patch in this message | | | Date | Wed, 10 Sep 2014 18:21:05 +0100 | From | Sudeep Holla <> | Subject | Re: [PATCH v4 09/11] ARM64: kernel: add support for cpu cache information |
| |
Hi Will,
Thanks for having a look this.
On 10/09/14 17:41, Will Deacon wrote: > Hi Sudeep, > > On Wed, Sep 03, 2014 at 06:00:15PM +0100, Sudeep Holla wrote: >> From: Sudeep Holla <sudeep.holla@arm.com> >> >> This patch adds support for cacheinfo on ARM64. >> >> On ARMv8, the cache hierarchy can be identified through Cache Level ID >> (CLIDR) register while the cache geometry is provided by Cache Size ID >> (CCSIDR) register. >> >> Since the architecture doesn't provide any way of detecting the cpus >> sharing particular cache, device tree is used for the same purpose. > > Out of interest, what's this actually for? Is there something useful in > userspace that will lap this out of sysfs? If so, it would be great if those > people could take these patches for a spin. >
I am not aware how the userspace makes use of this information in particular. Since it's already part of cacheinfo ABI on other architectures, I am just making sure we present some sane value deriving it from DT on ARM{32,64} platforms. For sure lscpu uses it, not aware of any other application.
>> diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c >> new file mode 100644 >> index 000000000000..a9cbf3b40a1f >> --- /dev/null >> +++ b/arch/arm64/kernel/cacheinfo.c >> @@ -0,0 +1,142 @@ >> +/* >> + * ARM64 cacheinfo support >> + * >> + * Copyright (C) 2014 ARM Ltd. >> + * All Rights Reserved >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any >> + * kind, whether express or implied; without even the implied warranty >> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <linux/bitops.h> >> +#include <linux/cacheinfo.h> >> +#include <linux/cpu.h> >> +#include <linux/compiler.h> >> +#include <linux/of.h> >> + >> +#include <asm/processor.h> >> + >> +#define MAX_CACHE_LEVEL 7 /* Max 7 level supported */ >> +/* Ctypen, bits[3(n - 1) + 2 : 3(n - 1)], for n = 1 to 7 */ >> +#define CLIDR_CTYPE_SHIFT(level) (3 * (level - 1)) >> +#define CLIDR_CTYPE_MASK(level) (7 << CLIDR_CTYPE_SHIFT(level)) >> +#define CLIDR_CTYPE(clidr, level) \ >> + (((clidr) & CLIDR_CTYPE_MASK(level)) >> CLIDR_CTYPE_SHIFT(level)) >> + >> +static inline enum cache_type get_cache_type(int level) >> +{ >> + u64 clidr; >> + >> + if (level > MAX_CACHE_LEVEL) >> + return CACHE_TYPE_NOCACHE; >> + asm volatile ("mrs %x0, clidr_el1" : "=r" (clidr)); >> + return CLIDR_CTYPE(clidr, level); >> +} >> + >> +/* >> + * NumSets, bits[27:13] - (Number of sets in cache) - 1 >> + * Associativity, bits[12:3] - (Associativity of cache) - 1 >> + * LineSize, bits[2:0] - (Log2(Number of words in cache line)) - 2 >> + */ >> +#define CCSIDR_WRITE_THROUGH BIT(31) >> +#define CCSIDR_WRITE_BACK BIT(30) >> +#define CCSIDR_READ_ALLOCATE BIT(29) >> +#define CCSIDR_WRITE_ALLOCATE BIT(28) >> +#define CCSIDR_LINESIZE_MASK 0x7 >> +#define CCSIDR_ASSOCIATIVITY_SHIFT 3 >> +#define CCSIDR_ASSOCIATIVITY_MASK 0x3FF >> +#define CCSIDR_NUMSETS_SHIFT 13 >> +#define CCSIDR_NUMSETS_MASK 0x7FF >> + >> +/* >> + * Which cache CCSIDR represents depends on CSSELR value >> + * Make sure no one else changes CSSELR during this >> + * smp_call_function_single prevents preemption for us >> + */ >> +static inline u32 get_ccsidr(u64 csselr) >> +{ >> + u64 ccsidr; >> + >> + /* Put value into CSSELR */ >> + asm volatile("msr csselr_el1, %x0" : : "r" (csselr)); >> + isb(); >> + /* Read result out of CCSIDR */ >> + asm volatile("mrs %x0, ccsidr_el1" : "=r" (ccsidr)); >> + >> + return (u32)ccsidr; > Since you're using %x0, can you just make ccsidr a u32? >
This was suggested by MarkR, I think u32 should be fine for reading back ccsidr. IIRC his concern was more when writing cssselr as GCC might leave stale data in upper 32-bit if we use u32 for csselr but had suggested to change both.
> Also, we have icache_get_ccsidr in kernel/cpuinfo.c already, as well as > some parsing constants in asm/cachetype.h. Can you try to clean up some of > the duplication/needless split please? (moving this helper and the #defines > out would be a good start). >
Agreed, I know and am already tracking recent changes from Ard, will clean up the duplication once it hits the mainline.
Regards, Sudeep
| |