lkml.org 
[lkml]   [2014]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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



\
 
 \ /
  Last update: 2014-09-10 20:01    [W:0.262 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site