[1/2] arm64: kvm: reuse existing cache type/info related macros

Message ID 1484909410-11673-1-git-send-email-sudeep.holla@arm.com
State New
Headers show
Series
  • [1/2] arm64: kvm: reuse existing cache type/info related macros
Related show

Commit Message

Sudeep Holla Jan. 20, 2017, 10:50 a.m.
We already have various macros related to cache type and bitfields in
CLIDR system register. We can replace some of the hardcoded values
here using those existing macros.

This patch reuses those existing cache type/info related macros and
replaces the hardcorded values. It also removes some of the comments
that become trivial with the macro names.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

---
 arch/arm64/include/asm/cachetype.h |  7 +++++++
 arch/arm64/kernel/cacheinfo.c      |  7 -------
 arch/arm64/kvm/sys_regs.c          | 27 +++++++++++++--------------
 3 files changed, 20 insertions(+), 21 deletions(-)

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Comments

Will Deacon Jan. 23, 2017, 6:17 p.m. | #1
On Fri, Jan 20, 2017 at 10:50:09AM +0000, Sudeep Holla wrote:
> We already have various macros related to cache type and bitfields in

> CLIDR system register. We can replace some of the hardcoded values

> here using those existing macros.

> 

> This patch reuses those existing cache type/info related macros and

> replaces the hardcorded values. It also removes some of the comments

> that become trivial with the macro names.

> 

> Cc: Catalin Marinas <catalin.marinas@arm.com>

> Cc: Will Deacon <will.deacon@arm.com>

> Cc: Christoffer Dall <christoffer.dall@linaro.org>

> Cc: Marc Zyngier <marc.zyngier@arm.com>

> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

> ---

>  arch/arm64/include/asm/cachetype.h |  7 +++++++

>  arch/arm64/kernel/cacheinfo.c      |  7 -------

>  arch/arm64/kvm/sys_regs.c          | 27 +++++++++++++--------------

>  3 files changed, 20 insertions(+), 21 deletions(-)


I assume both of these will go via kvm-arm.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Christoffer Dall Jan. 23, 2017, 9:05 p.m. | #2
On Fri, Jan 20, 2017 at 10:50:09AM +0000, Sudeep Holla wrote:
> We already have various macros related to cache type and bitfields in

> CLIDR system register. We can replace some of the hardcoded values

> here using those existing macros.

> 

> This patch reuses those existing cache type/info related macros and

> replaces the hardcorded values. It also removes some of the comments

> that become trivial with the macro names.

> 

> Cc: Catalin Marinas <catalin.marinas@arm.com>

> Cc: Will Deacon <will.deacon@arm.com>

> Cc: Christoffer Dall <christoffer.dall@linaro.org>

> Cc: Marc Zyngier <marc.zyngier@arm.com>

> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

> ---

>  arch/arm64/include/asm/cachetype.h |  7 +++++++

>  arch/arm64/kernel/cacheinfo.c      |  7 -------

>  arch/arm64/kvm/sys_regs.c          | 27 +++++++++++++--------------

>  3 files changed, 20 insertions(+), 21 deletions(-)

> 

> diff --git a/arch/arm64/include/asm/cachetype.h b/arch/arm64/include/asm/cachetype.h

> index f5588692f1d4..f58b5e3df6b8 100644

> --- a/arch/arm64/include/asm/cachetype.h

> +++ b/arch/arm64/include/asm/cachetype.h

> @@ -39,6 +39,13 @@

>  

>  extern unsigned long __icache_flags;

>  

> +#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))

> +

>  /*

>   * NumSets, bits[27:13] - (Number of sets in cache) - 1

>   * Associativity, bits[12:3] - (Associativity of cache) - 1

> diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c

> index 3f2250fc391b..a460208b08cf 100644

> --- a/arch/arm64/kernel/cacheinfo.c

> +++ b/arch/arm64/kernel/cacheinfo.c

> @@ -26,13 +26,6 @@

>  #include <asm/cachetype.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;

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c

> index 87e7e6608cd8..5dca1f10340f 100644

> --- a/arch/arm64/kvm/sys_regs.c

> +++ b/arch/arm64/kvm/sys_regs.c

> @@ -21,11 +21,13 @@

>   */

>  

>  #include <linux/bsearch.h>

> +#include <linux/cacheinfo.h>

>  #include <linux/kvm_host.h>

>  #include <linux/mm.h>

>  #include <linux/uaccess.h>

>  

>  #include <asm/cacheflush.h>

> +#include <asm/cachetype.h>

>  #include <asm/cputype.h>

>  #include <asm/debug-monitors.h>

>  #include <asm/esr.h>

> @@ -59,7 +61,7 @@

>  static u32 cache_levels;

>  

>  /* CSSELR values; used to index KVM_REG_ARM_DEMUX_ID_CCSIDR */

> -#define CSSELR_MAX 12

> +#define CSSELR_MAX 	((MAX_CACHE_LEVEL - 1) >> 1)


Did you mean '<< 1' here?

>  

>  /* Which cache CCSIDR represents depends on CSSELR value. */

>  static u32 get_ccsidr(u32 csselr)

> @@ -68,9 +70,7 @@ static u32 get_ccsidr(u32 csselr)

>  

>  	/* Make sure noone else changes CSSELR during this! */

>  	local_irq_disable();

> -	write_sysreg(csselr, csselr_el1);

> -	isb();

> -	ccsidr = read_sysreg(ccsidr_el1);

> +	ccsidr = cache_get_ccsidr(csselr);

>  	local_irq_enable();

>  

>  	return ccsidr;

> @@ -1960,19 +1960,18 @@ static bool is_valid_cache(u32 val)

>  		return false;

>  

>  	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */

> -	level = (val >> 1);

> -	ctype = (cache_levels >> (level * 3)) & 7;

> +	level = (val >> 1) + 1;

> +	ctype = CLIDR_CTYPE(cache_levels, level);

>  

>  	switch (ctype) {

> -	case 0: /* No cache */

> -		return false;

> -	case 1: /* Instruction cache only */

> -		return (val & 1);

> -	case 2: /* Data cache only */

> -	case 4: /* Unified cache */

> -		return !(val & 1);

> -	case 3: /* Separate instruction and data caches */

> +	case CACHE_TYPE_INST:

> +		return (val & CACHE_TYPE_INST);

> +	case CACHE_TYPE_DATA:

> +	case CACHE_TYPE_UNIFIED:

> +		return !(val & CACHE_TYPE_INST);

> +	case CACHE_TYPE_SEPARATE:

>  		return true;

> +	case CACHE_TYPE_NOCACHE:

>  	default: /* Reserved: we can't know instruction or data. */

>  		return false;

>  	}

> -- 

> 2.7.4

> 


Otherwise this looks correct to me.

Thanks,
-Christoffer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Sudeep Holla Jan. 24, 2017, 10:04 a.m. | #3
On 23/01/17 21:05, Christoffer Dall wrote:
> On Fri, Jan 20, 2017 at 10:50:09AM +0000, Sudeep Holla wrote:

>> We already have various macros related to cache type and bitfields in

>> CLIDR system register. We can replace some of the hardcoded values

>> here using those existing macros.

>>

>> This patch reuses those existing cache type/info related macros and

>> replaces the hardcorded values. It also removes some of the comments

>> that become trivial with the macro names.

>>

>> Cc: Catalin Marinas <catalin.marinas@arm.com>

>> Cc: Will Deacon <will.deacon@arm.com>

>> Cc: Christoffer Dall <christoffer.dall@linaro.org>

>> Cc: Marc Zyngier <marc.zyngier@arm.com>

>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

>> ---

>>  arch/arm64/include/asm/cachetype.h |  7 +++++++

>>  arch/arm64/kernel/cacheinfo.c      |  7 -------

>>  arch/arm64/kvm/sys_regs.c          | 27 +++++++++++++--------------

>>  3 files changed, 20 insertions(+), 21 deletions(-)

>>

>> diff --git a/arch/arm64/include/asm/cachetype.h b/arch/arm64/include/asm/cachetype.h

>> index f5588692f1d4..f58b5e3df6b8 100644

>> --- a/arch/arm64/include/asm/cachetype.h

>> +++ b/arch/arm64/include/asm/cachetype.h

>> @@ -39,6 +39,13 @@

>>  

>>  extern unsigned long __icache_flags;

>>  

>> +#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))

>> +

>>  /*

>>   * NumSets, bits[27:13] - (Number of sets in cache) - 1

>>   * Associativity, bits[12:3] - (Associativity of cache) - 1

>> diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c

>> index 3f2250fc391b..a460208b08cf 100644

>> --- a/arch/arm64/kernel/cacheinfo.c

>> +++ b/arch/arm64/kernel/cacheinfo.c

>> @@ -26,13 +26,6 @@

>>  #include <asm/cachetype.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;

>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c

>> index 87e7e6608cd8..5dca1f10340f 100644

>> --- a/arch/arm64/kvm/sys_regs.c

>> +++ b/arch/arm64/kvm/sys_regs.c

>> @@ -21,11 +21,13 @@

>>   */

>>  

>>  #include <linux/bsearch.h>

>> +#include <linux/cacheinfo.h>

>>  #include <linux/kvm_host.h>

>>  #include <linux/mm.h>

>>  #include <linux/uaccess.h>

>>  

>>  #include <asm/cacheflush.h>

>> +#include <asm/cachetype.h>

>>  #include <asm/cputype.h>

>>  #include <asm/debug-monitors.h>

>>  #include <asm/esr.h>

>> @@ -59,7 +61,7 @@

>>  static u32 cache_levels;

>>  

>>  /* CSSELR values; used to index KVM_REG_ARM_DEMUX_ID_CCSIDR */

>> -#define CSSELR_MAX 12

>> +#define CSSELR_MAX 	((MAX_CACHE_LEVEL - 1) >> 1)

> 

> Did you mean '<< 1' here?

> 


Ah right, sorry for the stupid mistake.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Patch hide | download patch | download mbox

diff --git a/arch/arm64/include/asm/cachetype.h b/arch/arm64/include/asm/cachetype.h
index f5588692f1d4..f58b5e3df6b8 100644
--- a/arch/arm64/include/asm/cachetype.h
+++ b/arch/arm64/include/asm/cachetype.h
@@ -39,6 +39,13 @@ 
 
 extern unsigned long __icache_flags;
 
+#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))
+
 /*
  * NumSets, bits[27:13] - (Number of sets in cache) - 1
  * Associativity, bits[12:3] - (Associativity of cache) - 1
diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
index 3f2250fc391b..a460208b08cf 100644
--- a/arch/arm64/kernel/cacheinfo.c
+++ b/arch/arm64/kernel/cacheinfo.c
@@ -26,13 +26,6 @@ 
 #include <asm/cachetype.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;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 87e7e6608cd8..5dca1f10340f 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -21,11 +21,13 @@ 
  */
 
 #include <linux/bsearch.h>
+#include <linux/cacheinfo.h>
 #include <linux/kvm_host.h>
 #include <linux/mm.h>
 #include <linux/uaccess.h>
 
 #include <asm/cacheflush.h>
+#include <asm/cachetype.h>
 #include <asm/cputype.h>
 #include <asm/debug-monitors.h>
 #include <asm/esr.h>
@@ -59,7 +61,7 @@ 
 static u32 cache_levels;
 
 /* CSSELR values; used to index KVM_REG_ARM_DEMUX_ID_CCSIDR */
-#define CSSELR_MAX 12
+#define CSSELR_MAX 	((MAX_CACHE_LEVEL - 1) >> 1)
 
 /* Which cache CCSIDR represents depends on CSSELR value. */
 static u32 get_ccsidr(u32 csselr)
@@ -68,9 +70,7 @@  static u32 get_ccsidr(u32 csselr)
 
 	/* Make sure noone else changes CSSELR during this! */
 	local_irq_disable();
-	write_sysreg(csselr, csselr_el1);
-	isb();
-	ccsidr = read_sysreg(ccsidr_el1);
+	ccsidr = cache_get_ccsidr(csselr);
 	local_irq_enable();
 
 	return ccsidr;
@@ -1960,19 +1960,18 @@  static bool is_valid_cache(u32 val)
 		return false;
 
 	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */
-	level = (val >> 1);
-	ctype = (cache_levels >> (level * 3)) & 7;
+	level = (val >> 1) + 1;
+	ctype = CLIDR_CTYPE(cache_levels, level);
 
 	switch (ctype) {
-	case 0: /* No cache */
-		return false;
-	case 1: /* Instruction cache only */
-		return (val & 1);
-	case 2: /* Data cache only */
-	case 4: /* Unified cache */
-		return !(val & 1);
-	case 3: /* Separate instruction and data caches */
+	case CACHE_TYPE_INST:
+		return (val & CACHE_TYPE_INST);
+	case CACHE_TYPE_DATA:
+	case CACHE_TYPE_UNIFIED:
+		return !(val & CACHE_TYPE_INST);
+	case CACHE_TYPE_SEPARATE:
 		return true;
+	case CACHE_TYPE_NOCACHE:
 	default: /* Reserved: we can't know instruction or data. */
 		return false;
 	}