[PATCH 0/2] MAAR SMP fix

[PATCH 0/2] MAAR SMP fix

[Paul Burton]

This series improves support for Memory Accessibility Attribute
Registers (MAARs) by printing their configuration during boot and by
applying the configuration to secondary CPUs, which have their own
independent MAAR state.

Paul Burton (2):
  MIPS: print MAAR configuration during boot
  MIPS: initialise MAARs on secondary CPUs

 arch/mips/include/asm/maar.h |  9 +++++++
 arch/mips/kernel/smp.c       |  2 ++
 arch/mips/mm/init.c          | 57 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 65 insertions(+), 3 deletions(-)

-- 
2.5.3

[PATCH 0/2] MAAR SMP fix

[Paul Burton]

This series improves support for Memory Accessibility Attribute
Registers (MAARs) by printing their configuration during boot and by
applying the configuration to secondary CPUs, which have their own
independent MAAR state.

Paul Burton (2):
  MIPS: print MAAR configuration during boot
  MIPS: initialise MAARs on secondary CPUs

 arch/mips/include/asm/maar.h |  9 +++++++
 arch/mips/kernel/smp.c       |  2 ++
 arch/mips/mm/init.c          | 57 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 65 insertions(+), 3 deletions(-)

-- 
2.5.3

[PATCH 1/2] MIPS: print MAAR configuration during boot

[Paul Burton]

Verifying that the MAAR configuration is as expected is useful when
debugging the performance of a system. Print out the memory regions
configured via MAAR along with their attributes.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/mm/init.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 66d0f49..cbef5c2 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -373,6 +373,7 @@ unsigned __weak platform_maar_init(unsigned num_pairs)
 static void maar_init(void)
 {
 	unsigned num_maars, used, i;
+	phys_addr_t lower, upper, attr;
 
 	if (!cpu_has_maar)
 		return;
@@ -395,6 +396,34 @@ static void maar_init(void)
 		write_c0_maar(0);
 		back_to_back_c0_hazard();
 	}
+
+	pr_info("MAAR configuration:\n");
+	for (i = 0; i < num_maars; i += 2) {
+		write_c0_maari(i);
+		back_to_back_c0_hazard();
+		upper = read_c0_maar();
+
+		write_c0_maari(i + 1);
+		back_to_back_c0_hazard();
+		lower = read_c0_maar();
+
+		attr = lower & upper;
+		lower = (lower & MIPS_MAAR_ADDR) << 4;
+		upper = ((upper & MIPS_MAAR_ADDR) << 4) | 0xffff;
+
+		pr_info("  [%d]: ", i / 2);
+		if (!(attr & MIPS_MAAR_V)) {
+			pr_cont("disabled\n");
+			continue;
+		}
+
+		pr_cont("%pa-%pa", &lower, &upper);
+
+		if (attr & MIPS_MAAR_S)
+			pr_cont(" speculate");
+
+		pr_cont("\n");
+	}
 }
 
 void __init mem_init(void)
-- 
2.5.3

[PATCH 1/2] MIPS: print MAAR configuration during boot

[Paul Burton]

Verifying that the MAAR configuration is as expected is useful when
debugging the performance of a system. Print out the memory regions
configured via MAAR along with their attributes.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/mm/init.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 66d0f49..cbef5c2 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -373,6 +373,7 @@ unsigned __weak platform_maar_init(unsigned num_pairs)
 static void maar_init(void)
 {
 	unsigned num_maars, used, i;
+	phys_addr_t lower, upper, attr;
 
 	if (!cpu_has_maar)
 		return;
@@ -395,6 +396,34 @@ static void maar_init(void)
 		write_c0_maar(0);
 		back_to_back_c0_hazard();
 	}
+
+	pr_info("MAAR configuration:\n");
+	for (i = 0; i < num_maars; i += 2) {
+		write_c0_maari(i);
+		back_to_back_c0_hazard();
+		upper = read_c0_maar();
+
+		write_c0_maari(i + 1);
+		back_to_back_c0_hazard();
+		lower = read_c0_maar();
+
+		attr = lower & upper;
+		lower = (lower & MIPS_MAAR_ADDR) << 4;
+		upper = ((upper & MIPS_MAAR_ADDR) << 4) | 0xffff;
+
+		pr_info("  [%d]: ", i / 2);
+		if (!(attr & MIPS_MAAR_V)) {
+			pr_cont("disabled\n");
+			continue;
+		}
+
+		pr_cont("%pa-%pa", &lower, &upper);
+
+		if (attr & MIPS_MAAR_S)
+			pr_cont(" speculate");
+
+		pr_cont("\n");
+	}
 }
 
 void __init mem_init(void)
-- 
2.5.3

[PATCH 2/2] MIPS: initialise MAARs on secondary CPUs

[Paul Burton]

MAARs should be initialised on each CPU (or rather, core) in the system
in order to achieve consistent behaviour & performance. Previously they
have only been initialised on the boot CPU which leads to performance
problems if tasks are later scheduled on a secondary CPU, particularly
if those tasks make use of unaligned vector accesses where some CPUs
don't handle any cases in hardware for non-speculative memory regions.
Fix this by recording the MAAR configuration from the boot CPU and
applying it to secondary CPUs as part of their bringup.

Reported-by: Doug Gilmore <doug.gilmore@imgtec.com>
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/include/asm/maar.h |  9 +++++++++
 arch/mips/kernel/smp.c       |  2 ++
 arch/mips/mm/init.c          | 28 +++++++++++++++++++++++++---
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/maar.h b/arch/mips/include/asm/maar.h
index b02891f..21d9607 100644
--- a/arch/mips/include/asm/maar.h
+++ b/arch/mips/include/asm/maar.h
@@ -66,6 +66,15 @@ static inline void write_maar_pair(unsigned idx, phys_addr_t lower,
 }
 
 /**
+ * maar_init() - initialise MAARs
+ *
+ * Performs initialisation of MAARs for the current CPU, making use of the
+ * platforms implementation of platform_maar_init where necessary and
+ * duplicating the setup it provides on secondary CPUs.
+ */
+extern void maar_init(void);
+
+/**
  * struct maar_config - MAAR configuration data
  * @lower:	The lowest address that the MAAR pair will affect. Must be
  *		aligned to a 2^16 byte boundary.
diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index a31896c..bd4385a 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -42,6 +42,7 @@
 #include <asm/mmu_context.h>
 #include <asm/time.h>
 #include <asm/setup.h>
+#include <asm/maar.h>
 
 cpumask_t cpu_callin_map;		/* Bitmask of started secondaries */
 
@@ -157,6 +158,7 @@ asmlinkage void start_secondary(void)
 	mips_clockevent_init();
 	mp_ops->init_secondary();
 	cpu_report();
+	maar_init();
 
 	/*
 	 * XXX parity protection should be folded in here when it's converted
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index cbef5c2..5f3c5b1 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -44,6 +44,7 @@
 #include <asm/pgalloc.h>
 #include <asm/tlb.h>
 #include <asm/fixmap.h>
+#include <asm/maar.h>
 
 /*
  * We have up to 8 empty zeroed pages so we can map one of the right colour
@@ -370,10 +371,14 @@ unsigned __weak platform_maar_init(unsigned num_pairs)
 	return num_configured;
 }
 
-static void maar_init(void)
+void maar_init(void)
 {
 	unsigned num_maars, used, i;
 	phys_addr_t lower, upper, attr;
+	static struct {
+		struct maar_config cfgs[3];
+		unsigned used;
+	} recorded = { { { 0 } }, 0 };
 
 	if (!cpu_has_maar)
 		return;
@@ -386,8 +391,14 @@ static void maar_init(void)
 	/* MAARs should be in pairs */
 	WARN_ON(num_maars % 2);
 
-	/* Configure the required MAARs */
-	used = platform_maar_init(num_maars / 2);
+	/* Set MAARs using values we recorded already */
+	if (recorded.used) {
+		used = maar_config(recorded.cfgs, recorded.used, num_maars / 2);
+		BUG_ON(used != recorded.used);
+	} else {
+		/* Configure the required MAARs */
+		used = platform_maar_init(num_maars / 2);
+	}
 
 	/* Disable any further MAARs */
 	for (i = (used * 2); i < num_maars; i++) {
@@ -397,6 +408,9 @@ static void maar_init(void)
 		back_to_back_c0_hazard();
 	}
 
+	if (recorded.used)
+		return;
+
 	pr_info("MAAR configuration:\n");
 	for (i = 0; i < num_maars; i += 2) {
 		write_c0_maari(i);
@@ -423,6 +437,14 @@ static void maar_init(void)
 			pr_cont(" speculate");
 
 		pr_cont("\n");
+
+		/* Record the setup for use on secondary CPUs */
+		if (used <= ARRAY_SIZE(recorded.cfgs)) {
+			recorded.cfgs[recorded.used].lower = lower;
+			recorded.cfgs[recorded.used].upper = upper;
+			recorded.cfgs[recorded.used].attrs = attr;
+			recorded.used++;
+		}
 	}
 }
 
-- 
2.5.3

[PATCH 2/2] MIPS: initialise MAARs on secondary CPUs

[Paul Burton]

MAARs should be initialised on each CPU (or rather, core) in the system
in order to achieve consistent behaviour & performance. Previously they
have only been initialised on the boot CPU which leads to performance
problems if tasks are later scheduled on a secondary CPU, particularly
if those tasks make use of unaligned vector accesses where some CPUs
don't handle any cases in hardware for non-speculative memory regions.
Fix this by recording the MAAR configuration from the boot CPU and
applying it to secondary CPUs as part of their bringup.

Reported-by: Doug Gilmore <doug.gilmore@imgtec.com>
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/include/asm/maar.h |  9 +++++++++
 arch/mips/kernel/smp.c       |  2 ++
 arch/mips/mm/init.c          | 28 +++++++++++++++++++++++++---
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/maar.h b/arch/mips/include/asm/maar.h
index b02891f..21d9607 100644
--- a/arch/mips/include/asm/maar.h
+++ b/arch/mips/include/asm/maar.h
@@ -66,6 +66,15 @@ static inline void write_maar_pair(unsigned idx, phys_addr_t lower,
 }
 
 /**
+ * maar_init() - initialise MAARs
+ *
+ * Performs initialisation of MAARs for the current CPU, making use of the
+ * platforms implementation of platform_maar_init where necessary and
+ * duplicating the setup it provides on secondary CPUs.
+ */
+extern void maar_init(void);
+
+/**
  * struct maar_config - MAAR configuration data
  * @lower:	The lowest address that the MAAR pair will affect. Must be
  *		aligned to a 2^16 byte boundary.
diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index a31896c..bd4385a 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -42,6 +42,7 @@
 #include <asm/mmu_context.h>
 #include <asm/time.h>
 #include <asm/setup.h>
+#include <asm/maar.h>
 
 cpumask_t cpu_callin_map;		/* Bitmask of started secondaries */
 
@@ -157,6 +158,7 @@ asmlinkage void start_secondary(void)
 	mips_clockevent_init();
 	mp_ops->init_secondary();
 	cpu_report();
+	maar_init();
 
 	/*
 	 * XXX parity protection should be folded in here when it's converted
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index cbef5c2..5f3c5b1 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -44,6 +44,7 @@
 #include <asm/pgalloc.h>
 #include <asm/tlb.h>
 #include <asm/fixmap.h>
+#include <asm/maar.h>
 
 /*
  * We have up to 8 empty zeroed pages so we can map one of the right colour
@@ -370,10 +371,14 @@ unsigned __weak platform_maar_init(unsigned num_pairs)
 	return num_configured;
 }
 
-static void maar_init(void)
+void maar_init(void)
 {
 	unsigned num_maars, used, i;
 	phys_addr_t lower, upper, attr;
+	static struct {
+		struct maar_config cfgs[3];
+		unsigned used;
+	} recorded = { { { 0 } }, 0 };
 
 	if (!cpu_has_maar)
 		return;
@@ -386,8 +391,14 @@ static void maar_init(void)
 	/* MAARs should be in pairs */
 	WARN_ON(num_maars % 2);
 
-	/* Configure the required MAARs */
-	used = platform_maar_init(num_maars / 2);
+	/* Set MAARs using values we recorded already */
+	if (recorded.used) {
+		used = maar_config(recorded.cfgs, recorded.used, num_maars / 2);
+		BUG_ON(used != recorded.used);
+	} else {
+		/* Configure the required MAARs */
+		used = platform_maar_init(num_maars / 2);
+	}
 
 	/* Disable any further MAARs */
 	for (i = (used * 2); i < num_maars; i++) {
@@ -397,6 +408,9 @@ static void maar_init(void)
 		back_to_back_c0_hazard();
 	}
 
+	if (recorded.used)
+		return;
+
 	pr_info("MAAR configuration:\n");
 	for (i = 0; i < num_maars; i += 2) {
 		write_c0_maari(i);
@@ -423,6 +437,14 @@ static void maar_init(void)
 			pr_cont(" speculate");
 
 		pr_cont("\n");
+
+		/* Record the setup for use on secondary CPUs */
+		if (used <= ARRAY_SIZE(recorded.cfgs)) {
+			recorded.cfgs[recorded.used].lower = lower;
+			recorded.cfgs[recorded.used].upper = upper;
+			recorded.cfgs[recorded.used].attrs = attr;
+			recorded.used++;
+		}
 	}
 }
 
-- 
2.5.3

Re: [PATCH 2/2] MIPS: initialise MAARs on secondary CPUs

[Markos Chandras]

On 09/22/2015 08:08 PM, Paul Burton wrote:
> MAARs should be initialised on each CPU (or rather, core) in the system
> in order to achieve consistent behaviour & performance. Previously they
> have only been initialised on the boot CPU which leads to performance
> problems if tasks are later scheduled on a secondary CPU, particularly
> if those tasks make use of unaligned vector accesses where some CPUs
> don't handle any cases in hardware for non-speculative memory regions.
> Fix this by recording the MAAR configuration from the boot CPU and
> applying it to secondary CPUs as part of their bringup.
> 
> Reported-by: Doug Gilmore <doug.gilmore@imgtec.com>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
> 
>  arch/mips/include/asm/maar.h |  9 +++++++++
>  arch/mips/kernel/smp.c       |  2 ++
>  arch/mips/mm/init.c          | 28 +++++++++++++++++++++++++---
>  3 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/include/asm/maar.h b/arch/mips/include/asm/maar.h
> index b02891f..21d9607 100644
> --- a/arch/mips/include/asm/maar.h
> +++ b/arch/mips/include/asm/maar.h
> @@ -66,6 +66,15 @@ static inline void write_maar_pair(unsigned idx, phys_addr_t lower,
>  }
>  
>  /**
> + * maar_init() - initialise MAARs
> + *
> + * Performs initialisation of MAARs for the current CPU, making use of the
> + * platforms implementation of platform_maar_init where necessary and
> + * duplicating the setup it provides on secondary CPUs.
> + */
> +extern void maar_init(void);
> +
> +/**
>   * struct maar_config - MAAR configuration data
>   * @lower:	The lowest address that the MAAR pair will affect. Must be
>   *		aligned to a 2^16 byte boundary.
> diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
> index a31896c..bd4385a 100644
> --- a/arch/mips/kernel/smp.c
> +++ b/arch/mips/kernel/smp.c
> @@ -42,6 +42,7 @@
>  #include <asm/mmu_context.h>
>  #include <asm/time.h>
>  #include <asm/setup.h>
> +#include <asm/maar.h>
>  
>  cpumask_t cpu_callin_map;		/* Bitmask of started secondaries */
>  
> @@ -157,6 +158,7 @@ asmlinkage void start_secondary(void)
>  	mips_clockevent_init();
>  	mp_ops->init_secondary();
>  	cpu_report();
> +	maar_init();
>  

Hi,

This breaks the ip27_defconfig in both upstream-sfr and linux-next

arch/mips/built-in.o: In function `start_secondary':
(.text+0x123e4): undefined reference to `maar_init'
Makefile:944: recipe for target 'vmlinux' failed


-- 
markos

Re: [PATCH 2/2] MIPS: initialise MAARs on secondary CPUs

[Markos Chandras]

On 09/22/2015 08:08 PM, Paul Burton wrote:
> MAARs should be initialised on each CPU (or rather, core) in the system
> in order to achieve consistent behaviour & performance. Previously they
> have only been initialised on the boot CPU which leads to performance
> problems if tasks are later scheduled on a secondary CPU, particularly
> if those tasks make use of unaligned vector accesses where some CPUs
> don't handle any cases in hardware for non-speculative memory regions.
> Fix this by recording the MAAR configuration from the boot CPU and
> applying it to secondary CPUs as part of their bringup.
> 
> Reported-by: Doug Gilmore <doug.gilmore@imgtec.com>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
> 
>  arch/mips/include/asm/maar.h |  9 +++++++++
>  arch/mips/kernel/smp.c       |  2 ++
>  arch/mips/mm/init.c          | 28 +++++++++++++++++++++++++---
>  3 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/include/asm/maar.h b/arch/mips/include/asm/maar.h
> index b02891f..21d9607 100644
> --- a/arch/mips/include/asm/maar.h
> +++ b/arch/mips/include/asm/maar.h
> @@ -66,6 +66,15 @@ static inline void write_maar_pair(unsigned idx, phys_addr_t lower,
>  }
>  
>  /**
> + * maar_init() - initialise MAARs
> + *
> + * Performs initialisation of MAARs for the current CPU, making use of the
> + * platforms implementation of platform_maar_init where necessary and
> + * duplicating the setup it provides on secondary CPUs.
> + */
> +extern void maar_init(void);
> +
> +/**
>   * struct maar_config - MAAR configuration data
>   * @lower:	The lowest address that the MAAR pair will affect. Must be
>   *		aligned to a 2^16 byte boundary.
> diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
> index a31896c..bd4385a 100644
> --- a/arch/mips/kernel/smp.c
> +++ b/arch/mips/kernel/smp.c
> @@ -42,6 +42,7 @@
>  #include <asm/mmu_context.h>
>  #include <asm/time.h>
>  #include <asm/setup.h>
> +#include <asm/maar.h>
>  
>  cpumask_t cpu_callin_map;		/* Bitmask of started secondaries */
>  
> @@ -157,6 +158,7 @@ asmlinkage void start_secondary(void)
>  	mips_clockevent_init();
>  	mp_ops->init_secondary();
>  	cpu_report();
> +	maar_init();
>  

Hi,

This breaks the ip27_defconfig in both upstream-sfr and linux-next

arch/mips/built-in.o: In function `start_secondary':
(.text+0x123e4): undefined reference to `maar_init'
Makefile:944: recipe for target 'vmlinux' failed


-- 
markos