[PATCH v2 1/3] arm64: fix typo in I-cache policy detection

[PATCH v2 1/3] arm64: fix typo in I-cache policy detection

[Ard Biesheuvel]

This removes an unfortunately placed semi-colon resulting in all instruction
caches being classified as AIVIVT.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
v2: add ack

 arch/arm64/kernel/cpuinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index f798f66634af..177169623026 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -49,7 +49,7 @@ static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
 
 	if (l1ip != ICACHE_POLICY_PIPT)
 		set_bit(ICACHEF_ALIASING, &__icache_flags);
-	if (l1ip == ICACHE_POLICY_AIVIVT);
+	if (l1ip == ICACHE_POLICY_AIVIVT)
 		set_bit(ICACHEF_AIVIVT, &__icache_flags);
 
 	pr_info("Detected %s I-cache on CPU%d\n", icache_policy_str[l1ip], cpu);
-- 
1.8.3.2

[PATCH v2 2/3] arm64: add helper functions to read I-cache attributes

[Ard Biesheuvel]

This adds helper functions and #defines to <asm/cachetype.h> to read the
line size and the number of sets from the level 1 instruction cache.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: put () around macro args, use 64-bit types for asm() mrs/msr calls

 arch/arm64/include/asm/cachetype.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/include/asm/cachetype.h b/arch/arm64/include/asm/cachetype.h
index 7a2e0762cb40..e59c0c25b307 100644
--- a/arch/arm64/include/asm/cachetype.h
+++ b/arch/arm64/include/asm/cachetype.h
@@ -39,6 +39,34 @@
 
 extern unsigned long __icache_flags;
 
+#define CCSIDR_EL1_LINESIZE_MASK	0x7
+#define CCSIDR_EL1_LINESIZE(x)		((x) & CCSIDR_EL1_LINESIZE_MASK)
+
+#define CCSIDR_EL1_NUMSETS_SHIFT	13
+#define CCSIDR_EL1_NUMSETS_MASK		(0x7fff << CCSIDR_EL1_NUMSETS_SHIFT)
+#define CCSIDR_EL1_NUMSETS(x) \
+	(((x) & CCSIDR_EL1_NUMSETS_MASK) >> CCSIDR_EL1_NUMSETS_SHIFT)
+
+static inline __attribute_const__ u64 icache_get_ccsidr(void)
+{
+	u64 ccsidr;
+
+	/* Select L1 I-cache and read its size ID register */
+	asm("msr csselr_el1, %1; isb; mrs %0, ccsidr_el1"
+	    : "=r"(ccsidr) : "r"(1L));
+	return ccsidr;
+}
+
+static inline int icache_get_linesize(void)
+{
+	return 16 << CCSIDR_EL1_LINESIZE(icache_get_ccsidr());
+}
+
+static inline int icache_get_numsets(void)
+{
+	return 1 + CCSIDR_EL1_NUMSETS(icache_get_ccsidr());
+}
+
 /*
  * Whilst the D-side always behaves as PIPT on AArch64, aliasing is
  * permitted in the I-cache.
-- 
1.8.3.2

[PATCH v2 2/3] arm64: add helper functions to read I-cache attributes

[Will Deacon]

On Tue, Aug 05, 2014 at 10:25:56AM +0100, Ard Biesheuvel wrote:
> This adds helper functions and #defines to <asm/cachetype.h> to read the
> line size and the number of sets from the level 1 instruction cache.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v2: put () around macro args, use 64-bit types for asm() mrs/msr calls
> 
>  arch/arm64/include/asm/cachetype.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cachetype.h b/arch/arm64/include/asm/cachetype.h
> index 7a2e0762cb40..e59c0c25b307 100644
> --- a/arch/arm64/include/asm/cachetype.h
> +++ b/arch/arm64/include/asm/cachetype.h
> @@ -39,6 +39,34 @@
>  
>  extern unsigned long __icache_flags;
>  
> +#define CCSIDR_EL1_LINESIZE_MASK	0x7
> +#define CCSIDR_EL1_LINESIZE(x)		((x) & CCSIDR_EL1_LINESIZE_MASK)
> +
> +#define CCSIDR_EL1_NUMSETS_SHIFT	13
> +#define CCSIDR_EL1_NUMSETS_MASK		(0x7fff << CCSIDR_EL1_NUMSETS_SHIFT)
> +#define CCSIDR_EL1_NUMSETS(x) \
> +	(((x) & CCSIDR_EL1_NUMSETS_MASK) >> CCSIDR_EL1_NUMSETS_SHIFT)
> +
> +static inline __attribute_const__ u64 icache_get_ccsidr(void)
> +{
> +	u64 ccsidr;
> +
> +	/* Select L1 I-cache and read its size ID register */
> +	asm("msr csselr_el1, %1; isb; mrs %0, ccsidr_el1"
> +	    : "=r"(ccsidr) : "r"(1L));
> +	return ccsidr;

Is it worth having a WARN_ON(preemptible()) here?

Will

[PATCH v2 2/3] arm64: add helper functions to read I-cache attributes

[Ard Biesheuvel]

On 6 August 2014 15:00, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Aug 05, 2014 at 10:25:56AM +0100, Ard Biesheuvel wrote:
>> This adds helper functions and #defines to <asm/cachetype.h> to read the
>> line size and the number of sets from the level 1 instruction cache.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> v2: put () around macro args, use 64-bit types for asm() mrs/msr calls
>>
>>  arch/arm64/include/asm/cachetype.h | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/cachetype.h b/arch/arm64/include/asm/cachetype.h
>> index 7a2e0762cb40..e59c0c25b307 100644
>> --- a/arch/arm64/include/asm/cachetype.h
>> +++ b/arch/arm64/include/asm/cachetype.h
>> @@ -39,6 +39,34 @@
>>
>>  extern unsigned long __icache_flags;
>>
>> +#define CCSIDR_EL1_LINESIZE_MASK     0x7
>> +#define CCSIDR_EL1_LINESIZE(x)               ((x) & CCSIDR_EL1_LINESIZE_MASK)
>> +
>> +#define CCSIDR_EL1_NUMSETS_SHIFT     13
>> +#define CCSIDR_EL1_NUMSETS_MASK              (0x7fff << CCSIDR_EL1_NUMSETS_SHIFT)
>> +#define CCSIDR_EL1_NUMSETS(x) \
>> +     (((x) & CCSIDR_EL1_NUMSETS_MASK) >> CCSIDR_EL1_NUMSETS_SHIFT)
>> +
>> +static inline __attribute_const__ u64 icache_get_ccsidr(void)
>> +{
>> +     u64 ccsidr;
>> +
>> +     /* Select L1 I-cache and read its size ID register */
>> +     asm("msr csselr_el1, %1; isb; mrs %0, ccsidr_el1"
>> +         : "=r"(ccsidr) : "r"(1L));
>> +     return ccsidr;
>
> Is it worth having a WARN_ON(preemptible()) here?
>

Sure, why not.

[PATCH v2 2/3] arm64: add helper functions to read I-cache attributes

[Ard Biesheuvel]

On 6 August 2014 15:17, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 6 August 2014 15:00, Will Deacon <will.deacon@arm.com> wrote:
>> On Tue, Aug 05, 2014 at 10:25:56AM +0100, Ard Biesheuvel wrote:
>>> This adds helper functions and #defines to <asm/cachetype.h> to read the
>>> line size and the number of sets from the level 1 instruction cache.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> v2: put () around macro args, use 64-bit types for asm() mrs/msr calls
>>>
>>>  arch/arm64/include/asm/cachetype.h | 28 ++++++++++++++++++++++++++++
>>>  1 file changed, 28 insertions(+)
>>>
>>> diff --git a/arch/arm64/include/asm/cachetype.h b/arch/arm64/include/asm/cachetype.h
>>> index 7a2e0762cb40..e59c0c25b307 100644
>>> --- a/arch/arm64/include/asm/cachetype.h
>>> +++ b/arch/arm64/include/asm/cachetype.h
>>> @@ -39,6 +39,34 @@
>>>
>>>  extern unsigned long __icache_flags;
>>>
>>> +#define CCSIDR_EL1_LINESIZE_MASK     0x7
>>> +#define CCSIDR_EL1_LINESIZE(x)               ((x) & CCSIDR_EL1_LINESIZE_MASK)
>>> +
>>> +#define CCSIDR_EL1_NUMSETS_SHIFT     13
>>> +#define CCSIDR_EL1_NUMSETS_MASK              (0x7fff << CCSIDR_EL1_NUMSETS_SHIFT)
>>> +#define CCSIDR_EL1_NUMSETS(x) \
>>> +     (((x) & CCSIDR_EL1_NUMSETS_MASK) >> CCSIDR_EL1_NUMSETS_SHIFT)
>>> +
>>> +static inline __attribute_const__ u64 icache_get_ccsidr(void)
>>> +{
>>> +     u64 ccsidr;
>>> +
>>> +     /* Select L1 I-cache and read its size ID register */
>>> +     asm("msr csselr_el1, %1; isb; mrs %0, ccsidr_el1"
>>> +         : "=r"(ccsidr) : "r"(1L));
>>> +     return ccsidr;
>>
>> Is it worth having a WARN_ON(preemptible()) here?
>>
>
> Sure, why not.

... if it weren't for the fact that this triggers recursive header
inclusion hell

  CC      kernel/bounds.s
In file included from /home/ard/linux-2.6/include/asm-generic/preempt.h:4:0,
                 from arch/arm64/include/generated/asm/preempt.h:1,
                 from /home/ard/linux-2.6/include/linux/preempt.h:18,
                 from /home/ard/linux-2.6/arch/arm64/include/asm/cachetype.h:21,
                 from /home/ard/linux-2.6/arch/arm64/include/asm/cache.h:19,
                 from /home/ard/linux-2.6/include/linux/cache.h:5,
                 from /home/ard/linux-2.6/include/linux/printk.h:8,
                 from /home/ard/linux-2.6/include/linux/kernel.h:13,
                 from /home/ard/linux-2.6/include/asm-generic/bug.h:13,
                 from arch/arm64/include/generated/asm/bug.h:1,
                 from /home/ard/linux-2.6/include/linux/bug.h:4,
                 from /home/ard/linux-2.6/include/linux/page-flags.h:9,
                 from /home/ard/linux-2.6/kernel/bounds.c:9:
/home/ard/linux-2.6/include/linux/thread_info.h: In function
?set_restore_sigmask?:
/home/ard/linux-2.6/include/linux/thread_info.h:128:2: error: implicit
declaration of function ?WARN_ON?
[-Werror=implicit-function-declaration]
  WARN_ON(!test_thread_flag(TIF_SIGPENDING));
  ^
In file included from /home/ard/linux-2.6/arch/arm64/include/asm/cache.h:19:0,
                 from /home/ard/linux-2.6/include/linux/cache.h:5,
                 from /home/ard/linux-2.6/include/linux/printk.h:8,
                 from /home/ard/linux-2.6/include/linux/kernel.h:13,
                 from /home/ard/linux-2.6/include/asm-generic/bug.h:13,
                 from arch/arm64/include/generated/asm/bug.h:1,
                 from /home/ard/linux-2.6/include/linux/bug.h:4,
                 from /home/ard/linux-2.6/include/linux/page-flags.h:9,
                 from /home/ard/linux-2.6/kernel/bounds.c:9:
/home/ard/linux-2.6/arch/arm64/include/asm/cachetype.h: In function
?icache_get_ccsidr?:
/home/ard/linux-2.6/arch/arm64/include/asm/cachetype.h:59:2: error:
implicit declaration of function ?preemptible?
[-Werror=implicit-function-declaration]
  WARN_ON(preemptible());
  ^

i.e., linux/bug,h and linux/preempt.h already implicitly #include
cachetype.h, so including the former from the latter to import the
declaration of WARN_ON() and/or preemptible respectively produces this
error.

-- 
Ard.

[PATCH v2 2/3] arm64: add helper functions to read I-cache attributes

[Will Deacon]

On Wed, Aug 06, 2014 at 02:27:55PM +0100, Ard Biesheuvel wrote:
> On 6 August 2014 15:17, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 6 August 2014 15:00, Will Deacon <will.deacon@arm.com> wrote:
> >> On Tue, Aug 05, 2014 at 10:25:56AM +0100, Ard Biesheuvel wrote:
> >>> This adds helper functions and #defines to <asm/cachetype.h> to read the
> >>> line size and the number of sets from the level 1 instruction cache.
> >>>
> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>> ---

[...]

> >>> +static inline __attribute_const__ u64 icache_get_ccsidr(void)
> >>> +{
> >>> +     u64 ccsidr;
> >>> +
> >>> +     /* Select L1 I-cache and read its size ID register */
> >>> +     asm("msr csselr_el1, %1; isb; mrs %0, ccsidr_el1"
> >>> +         : "=r"(ccsidr) : "r"(1L));
> >>> +     return ccsidr;
> >>
> >> Is it worth having a WARN_ON(preemptible()) here?
> >>
> >
> > Sure, why not.
> 
> ... if it weren't for the fact that this triggers recursive header
> inclusion hell
> 
>   CC      kernel/bounds.s
> In file included from /home/ard/linux-2.6/include/asm-generic/preempt.h:4:0,
>                  from arch/arm64/include/generated/asm/preempt.h:1,
>                  from /home/ard/linux-2.6/include/linux/preempt.h:18,
>                  from /home/ard/linux-2.6/arch/arm64/include/asm/cachetype.h:21,

[...]

> i.e., linux/bug,h and linux/preempt.h already implicitly #include
> cachetype.h, so including the former from the latter to import the
> declaration of WARN_ON() and/or preemptible respectively produces this
> error.

Damn, that's a real shame. I'm always dubious about adding code like this
which isn't obviously broken from preemptible context when you're just
looking at the function name.

Ho-hum.

Will

[PATCH v2 2/3] arm64: add helper functions to read I-cache attributes

[Ard Biesheuvel]

On 6 August 2014 16:34, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Aug 06, 2014 at 02:27:55PM +0100, Ard Biesheuvel wrote:
>> On 6 August 2014 15:17, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > On 6 August 2014 15:00, Will Deacon <will.deacon@arm.com> wrote:
>> >> On Tue, Aug 05, 2014 at 10:25:56AM +0100, Ard Biesheuvel wrote:
>> >>> This adds helper functions and #defines to <asm/cachetype.h> to read the
>> >>> line size and the number of sets from the level 1 instruction cache.
>> >>>
>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >>> ---
>
> [...]
>
>> >>> +static inline __attribute_const__ u64 icache_get_ccsidr(void)
>> >>> +{
>> >>> +     u64 ccsidr;
>> >>> +
>> >>> +     /* Select L1 I-cache and read its size ID register */
>> >>> +     asm("msr csselr_el1, %1; isb; mrs %0, ccsidr_el1"
>> >>> +         : "=r"(ccsidr) : "r"(1L));
>> >>> +     return ccsidr;
>> >>
>> >> Is it worth having a WARN_ON(preemptible()) here?
>> >>
>> >
>> > Sure, why not.
>>
>> ... if it weren't for the fact that this triggers recursive header
>> inclusion hell
>>
>>   CC      kernel/bounds.s
>> In file included from /home/ard/linux-2.6/include/asm-generic/preempt.h:4:0,
>>                  from arch/arm64/include/generated/asm/preempt.h:1,
>>                  from /home/ard/linux-2.6/include/linux/preempt.h:18,
>>                  from /home/ard/linux-2.6/arch/arm64/include/asm/cachetype.h:21,
>
> [...]
>
>> i.e., linux/bug,h and linux/preempt.h already implicitly #include
>> cachetype.h, so including the former from the latter to import the
>> declaration of WARN_ON() and/or preemptible respectively produces this
>> error.
>
> Damn, that's a real shame. I'm always dubious about adding code like this
> which isn't obviously broken from preemptible context when you're just
> looking at the function name.
>
> Ho-hum.
>

Moving icache_get_ccsidr() into cpuinfo.c does work, as in the patch below

diff --git a/arch/arm64/include/asm/cachetype.h
b/arch/arm64/include/asm/cachetype.h
index 7a2e0762cb40..4c631a0a3609 100644
--- a/arch/arm64/include/asm/cachetype.h
+++ b/arch/arm64/include/asm/cachetype.h
@@ -39,6 +39,26 @@

 extern unsigned long __icache_flags;

+#define CCSIDR_EL1_LINESIZE_MASK 0x7
+#define CCSIDR_EL1_LINESIZE(x) ((x) & CCSIDR_EL1_LINESIZE_MASK)
+
+#define CCSIDR_EL1_NUMSETS_SHIFT 13
+#define CCSIDR_EL1_NUMSETS_MASK (0x7fff << CCSIDR_EL1_NUMSETS_SHIFT)
+#define CCSIDR_EL1_NUMSETS(x) \
+ (((x) & CCSIDR_EL1_NUMSETS_MASK) >> CCSIDR_EL1_NUMSETS_SHIFT)
+
+extern u64 __attribute_const__ icache_get_ccsidr(void);
+
+static inline int icache_get_linesize(void)
+{
+ return 16 << CCSIDR_EL1_LINESIZE(icache_get_ccsidr());
+}
+
+static inline int icache_get_numsets(void)
+{
+ return 1 + CCSIDR_EL1_NUMSETS(icache_get_ccsidr());
+}
+
 /*
  * Whilst the D-side always behaves as PIPT on AArch64, aliasing is
  * permitted in the I-cache.
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index f798f66634af..319255ff536d 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -20,8 +20,10 @@
 #include <asm/cputype.h>

 #include <linux/bitops.h>
+#include <linux/bug.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/preempt.h>
 #include <linux/printk.h>
 #include <linux/smp.h>

@@ -190,3 +192,15 @@ void __init cpuinfo_store_boot_cpu(void)

  boot_cpu_data = *info;
 }
+
+u64 __attribute_const__ icache_get_ccsidr(void)
+{
+ u64 ccsidr;
+
+ WARN_ON(preemptible());
+
+ /* Select L1 I-cache and read its size ID register */
+ asm("msr csselr_el1, %1; isb; mrs %0, ccsidr_el1"
+    : "=r"(ccsidr) : "r"(1L));
+ return ccsidr;
+}

[PATCH v2 3/3] arm64: don't flag non-aliasing VIPT I-caches as aliasing

[Ard Biesheuvel]

VIPT caches are non-aliasing if the index is derived from address bits that
are always equal between VA and PA. Classifying these as aliasing results in
unnecessary flushing which may hurt performance.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/cpuinfo.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 177169623026..eb993cbdacf7 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -47,7 +47,13 @@ static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
 	unsigned int cpu = smp_processor_id();
 	u32 l1ip = CTR_L1IP(info->reg_ctr);
 
-	if (l1ip != ICACHE_POLICY_PIPT)
+	/*
+	 * VIPT caches are non-aliasing if the VA always equals the PA in all
+	 * bit positions that are covered by the index, i.e., if num_sets_shift
+	 * is less than or equal to PAGE_SHIFT minus line_size_shift.
+	 */
+	if (l1ip != ICACHE_POLICY_PIPT && !(l1ip == ICACHE_POLICY_VIPT &&
+	    icache_get_linesize() * icache_get_numsets() <= PAGE_SIZE))
 		set_bit(ICACHEF_ALIASING, &__icache_flags);
 	if (l1ip == ICACHE_POLICY_AIVIVT)
 		set_bit(ICACHEF_AIVIVT, &__icache_flags);
-- 
1.8.3.2

[PATCH v2 1/3] arm64: fix typo in I-cache policy detection

[Will Deacon]

On Tue, Aug 05, 2014 at 10:25:55AM +0100, Ard Biesheuvel wrote:
> This removes an unfortunately placed semi-colon resulting in all instruction
> caches being classified as AIVIVT.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> ---
> v2: add ack

Cheers Ard, I'll take this patch into our fixes branch.

Will