[PATCH 0/2] ARM: uniphier: updates for Linux 4.7-rc1

[PATCH 0/2] ARM: uniphier: updates for Linux 4.7-rc1

[Masahiro Yamada]

Hi Arnd, Olof

Two patches for UniPhier SMP updates.

Please get them into ASOC.



Masahiro Yamada (2):
  ARM: uniphier: fix up cache ops broadcast of ACTLR
  ARM: uniphier: initialize outer cache for secondary CPUs

 arch/arm/include/asm/hardware/cache-uniphier.h |  5 +++++
 arch/arm/mach-uniphier/platsmp.c               | 21 +++++++++++++++++++++
 arch/arm/mm/cache-uniphier.c                   | 19 ++++++++++++++++---
 3 files changed, 42 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH 1/2] ARM: uniphier: fix up cache ops broadcast of ACTLR

[Masahiro Yamada]

The Boot ROM of the UniPhier ARMv7 SoCs sets ACTLR (Auxiliary Control
Register) to different values for different secure states:

[1] Set ACTLR to 0x41 for Non-secure boot
[2] Set ACTLR to 0x40 for Secure boot

[1] is okay, but [2] is a problem.  Because of commit 1b3a02eb4523
("ARMv7: Check whether the SMP/nAMP mode was already enabled"),
if bit 6 (SMP bit) is already set, the kernel skips the ACTLR setting.
In that case, bit 0 (FW bit) is never set, so cache ops is not
broadcasted, causing a cache coherency problem.

To solve the problem, this commit sets the bit 0 of ACTLR if the bit 4
has already been set.  This change is harmless for [1] because the
Boot ROM has already set NSACR (Non-secure Access Control Register)
bit 18 (NS_SMP bit) before switching to Non-secure state in order to
allow write access to the ACTLR.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm/mach-uniphier/platsmp.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/mach-uniphier/platsmp.c b/arch/arm/mach-uniphier/platsmp.c
index db04142..285b684 100644
--- a/arch/arm/mach-uniphier/platsmp.c
+++ b/arch/arm/mach-uniphier/platsmp.c
@@ -170,6 +170,18 @@ static int __init uniphier_smp_enable_scu(void)
 	return 0;
 }
 
+static void __init uniphier_smp_fixup_cache_broadcast(void)
+{
+	u32 tmp;
+
+	asm volatile(
+		"mrc	p15, 0, %0, c1, c0, 1\n"
+		"tst	%0, #(1 << 6)\n"
+		"orrne	%0, #(1 << 0)\n"
+		"mcr	p15, 0, %0, c1, c0, 1\n"
+		: "=r" (tmp) : : "memory", "cc");
+}
+
 static void __init uniphier_smp_prepare_cpus(unsigned int max_cpus)
 {
 	static cpumask_t only_cpu_0 = { CPU_BITS_CPU0 };
@@ -183,6 +195,8 @@ static void __init uniphier_smp_prepare_cpus(unsigned int max_cpus)
 	if (ret)
 		goto err;
 
+	uniphier_smp_fixup_cache_broadcast();
+
 	return;
 err:
 	pr_warn("disabling SMP\n");
@@ -209,9 +223,15 @@ static int __init uniphier_smp_boot_secondary(unsigned int cpu,
 	return 0;
 }
 
+static void __init uniphier_smp_secondary_init(unsigned int cpu)
+{
+	uniphier_smp_fixup_cache_broadcast();
+}
+
 static const struct smp_operations uniphier_smp_ops __initconst = {
 	.smp_prepare_cpus	= uniphier_smp_prepare_cpus,
 	.smp_boot_secondary	= uniphier_smp_boot_secondary,
+	.smp_secondary_init	= uniphier_smp_secondary_init,
 };
 CPU_METHOD_OF_DECLARE(uniphier_smp, "socionext,uniphier-smp",
 		      &uniphier_smp_ops);
-- 
1.9.1

[PATCH 1/2] ARM: uniphier: fix up cache ops broadcast of ACTLR

[Russell King - ARM Linux]

On Tue, Mar 29, 2016 at 10:38:23AM +0900, Masahiro Yamada wrote:
> The Boot ROM of the UniPhier ARMv7 SoCs sets ACTLR (Auxiliary Control
> Register) to different values for different secure states:
> 
> [1] Set ACTLR to 0x41 for Non-secure boot
> [2] Set ACTLR to 0x40 for Secure boot
> 
> [1] is okay, but [2] is a problem.  Because of commit 1b3a02eb4523
> ("ARMv7: Check whether the SMP/nAMP mode was already enabled"),
> if bit 6 (SMP bit) is already set, the kernel skips the ACTLR setting.
> In that case, bit 0 (FW bit) is never set, so cache ops is not
> broadcasted, causing a cache coherency problem.
> 
> To solve the problem, this commit sets the bit 0 of ACTLR if the bit 4
> has already been set.  This change is harmless for [1] because the
> Boot ROM has already set NSACR (Non-secure Access Control Register)
> bit 18 (NS_SMP bit) before switching to Non-secure state in order to
> allow write access to the ACTLR.

The test in proc-v7.S is too weak, we should probably tighten it to
prevent these kinds of problems, iow:

 arch/arm/mm/proc-v7.S | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 0f8963a7e7d9..6fcaac8e200f 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -281,12 +281,12 @@ __v7_ca17mp_setup:
 	bl      v7_invalidate_l1
 	ldmia	r12, {r1-r6, lr}
 #ifdef CONFIG_SMP
+	orr	r10, r10, #(1 << 6)		@ Enable SMP/nAMP mode
 	ALT_SMP(mrc	p15, 0, r0, c1, c0, 1)
-	ALT_UP(mov	r0, #(1 << 6))		@ fake it for UP
-	tst	r0, #(1 << 6)			@ SMP/nAMP mode enabled?
-	orreq	r0, r0, #(1 << 6)		@ Enable SMP/nAMP mode
-	orreq	r0, r0, r10			@ Enable CPU-specific SMP bits
-	mcreq	p15, 0, r0, c1, c0, 1
+	ALT_UP(mov	r0, r10)		@ fake it for UP
+	orr	r10, r10, r0			@ Set required bits
+	teq	r10, r0				@ Were they already set?
+	mcrne	p15, 0, r10, c1, c0, 1		@ No, update register
 #endif
 	b	__v7_setup_cont
 


-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 1/2] ARM: uniphier: fix up cache ops broadcast of ACTLR

[Masahiro Yamada]

Hi Russell,

2016-03-29 19:03 GMT+09:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, Mar 29, 2016 at 10:38:23AM +0900, Masahiro Yamada wrote:
>> The Boot ROM of the UniPhier ARMv7 SoCs sets ACTLR (Auxiliary Control
>> Register) to different values for different secure states:
>>
>> [1] Set ACTLR to 0x41 for Non-secure boot
>> [2] Set ACTLR to 0x40 for Secure boot
>>
>> [1] is okay, but [2] is a problem.  Because of commit 1b3a02eb4523
>> ("ARMv7: Check whether the SMP/nAMP mode was already enabled"),
>> if bit 6 (SMP bit) is already set, the kernel skips the ACTLR setting.
>> In that case, bit 0 (FW bit) is never set, so cache ops is not
>> broadcasted, causing a cache coherency problem.
>>
>> To solve the problem, this commit sets the bit 0 of ACTLR if the bit 4
>> has already been set.  This change is harmless for [1] because the
>> Boot ROM has already set NSACR (Non-secure Access Control Register)
>> bit 18 (NS_SMP bit) before switching to Non-secure state in order to
>> allow write access to the ACTLR.
>
> The test in proc-v7.S is too weak, we should probably tighten it to
> prevent these kinds of problems, iow:
>
>  arch/arm/mm/proc-v7.S | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index 0f8963a7e7d9..6fcaac8e200f 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -281,12 +281,12 @@ __v7_ca17mp_setup:
>         bl      v7_invalidate_l1
>         ldmia   r12, {r1-r6, lr}
>  #ifdef CONFIG_SMP
> +       orr     r10, r10, #(1 << 6)             @ Enable SMP/nAMP mode
>         ALT_SMP(mrc     p15, 0, r0, c1, c0, 1)
> -       ALT_UP(mov      r0, #(1 << 6))          @ fake it for UP
> -       tst     r0, #(1 << 6)                   @ SMP/nAMP mode enabled?
> -       orreq   r0, r0, #(1 << 6)               @ Enable SMP/nAMP mode
> -       orreq   r0, r0, r10                     @ Enable CPU-specific SMP bits
> -       mcreq   p15, 0, r0, c1, c0, 1
> +       ALT_UP(mov      r0, r10)                @ fake it for UP
> +       orr     r10, r10, r0                    @ Set required bits
> +       teq     r10, r0                         @ Were they already set?
> +       mcrne   p15, 0, r10, c1, c0, 1          @ No, update register
>  #endif
>         b       __v7_setup_cont
>
>
>

I tested it on some of my Cortex-A9 based boards.
(all the combinations of SMP/UP SoC and Secure/Non-secure boot)
and your patch worked fine!

Could you send it as a patch with git-log, please?
Please feel free to add my
Tested-by: Masahiro Yamada <yamada.masahiro@socionext.com>


I retract my crap patch.

Thank you!


-- 
Best Regards
Masahiro Yamada

[PATCH 2/2] ARM: uniphier: initialize outer cache for secondary CPUs

[Masahiro Yamada]

Some parts of this outer cache need per-CPU initialization.  The
registers for controlling active ways are banked for each CPU.

Each secondary CPU should activate ways at its boot-up.  Otherwise,
the data in the outer cache are not refilled in case of cache miss
from secondary CPUs, making data access extremely inefficient.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm/include/asm/hardware/cache-uniphier.h |  5 +++++
 arch/arm/mach-uniphier/platsmp.c               |  1 +
 arch/arm/mm/cache-uniphier.c                   | 19 ++++++++++++++++---
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/hardware/cache-uniphier.h b/arch/arm/include/asm/hardware/cache-uniphier.h
index 102e3fb..6bfa9d5 100644
--- a/arch/arm/include/asm/hardware/cache-uniphier.h
+++ b/arch/arm/include/asm/hardware/cache-uniphier.h
@@ -19,6 +19,7 @@
 
 #ifdef CONFIG_CACHE_UNIPHIER
 int uniphier_cache_init(void);
+void uniphier_cache_secondary_init(void);
 int uniphier_cache_l2_is_enabled(void);
 void uniphier_cache_l2_touch_range(unsigned long start, unsigned long end);
 void uniphier_cache_l2_set_locked_ways(u32 way_mask);
@@ -28,6 +29,10 @@ static inline int uniphier_cache_init(void)
 	return -ENODEV;
 }
 
+static inline void uniphier_cache_secondary_init(void)
+{
+}
+
 static inline int uniphier_cache_l2_is_enabled(void)
 {
 	return 0;
diff --git a/arch/arm/mach-uniphier/platsmp.c b/arch/arm/mach-uniphier/platsmp.c
index 285b684..80c04cf 100644
--- a/arch/arm/mach-uniphier/platsmp.c
+++ b/arch/arm/mach-uniphier/platsmp.c
@@ -226,6 +226,7 @@ static int __init uniphier_smp_boot_secondary(unsigned int cpu,
 static void __init uniphier_smp_secondary_init(unsigned int cpu)
 {
 	uniphier_smp_fixup_cache_broadcast();
+	uniphier_cache_secondary_init();
 }
 
 static const struct smp_operations uniphier_smp_ops __initconst = {
diff --git a/arch/arm/mm/cache-uniphier.c b/arch/arm/mm/cache-uniphier.c
index a6fa7b7..4e6f352 100644
--- a/arch/arm/mm/cache-uniphier.c
+++ b/arch/arm/mm/cache-uniphier.c
@@ -314,16 +314,24 @@ static void uniphier_cache_disable(void)
 	uniphier_cache_flush_all();
 }
 
+static void __init uniphier_cache_activate_all_ways(void)
+{
+	struct uniphier_cache_data *data;
+
+	list_for_each_entry(data, &uniphier_cache_list, list)
+		__uniphier_cache_set_locked_ways(data, 0);
+}
+
 static void __init uniphier_cache_enable(void)
 {
 	struct uniphier_cache_data *data;
 
 	uniphier_cache_inv_all();
 
-	list_for_each_entry(data, &uniphier_cache_list, list) {
+	list_for_each_entry(data, &uniphier_cache_list, list)
 		__uniphier_cache_enable(data, true);
-		__uniphier_cache_set_locked_ways(data, 0);
-	}
+
+	uniphier_cache_activate_all_ways();
 }
 
 static void uniphier_cache_sync(void)
@@ -542,3 +550,8 @@ int __init uniphier_cache_init(void)
 
 	return ret;
 }
+
+void uniphier_cache_secondary_init(void)
+{
+	uniphier_cache_activate_all_ways();
+}
-- 
1.9.1

[PATCH 2/2] ARM: uniphier: initialize outer cache for secondary CPUs

[kbuild test robot]

Hi Masahiro,

[auto build test WARNING on arm-soc/for-next]
[also build test WARNING on v4.6-rc1 next-20160329]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/ARM-uniphier-fix-up-cache-ops-broadcast-of-ACTLR/20160329-094052
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git for-next
config: arm-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

>> WARNING: vmlinux.o(.text+0x557b4): Section mismatch in reference from the function uniphier_cache_secondary_init() to the function .init.text:uniphier_cache_activate_all_ways()
   The function uniphier_cache_secondary_init() references
   the function __init uniphier_cache_activate_all_ways().
   This is often because uniphier_cache_secondary_init lacks a __init
   annotation or the annotation of uniphier_cache_activate_all_ways is wrong.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 56597 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160329/fcd77fe1/attachment-0001.obj>

[PATCH 2/2] ARM: uniphier: initialize outer cache for secondary CPUs

[Arnd Bergmann]

On Tuesday 29 March 2016 15:59:00 kbuild test robot wrote:
> 
> All warnings (new ones prefixed by >>):
> 
> >> WARNING: vmlinux.o(.text+0x557b4): Section mismatch in reference from the function uniphier_cache_secondary_init() to the function .init.text:uniphier_cache_activate_all_ways()
>    The function uniphier_cache_secondary_init() references
>    the function __init uniphier_cache_activate_all_ways().
>    This is often because uniphier_cache_secondary_init lacks a __init
>    annotation or the annotation of uniphier_cache_activate_all_ways is wrong.
> 


I guess the former: uniphier_cache_secondary_init should be __init, as it will
only be run at boot time.

Please resend both patches.

	Arnd

[PATCH 2/2] ARM: uniphier: initialize outer cache for secondary CPUs

[Masahiro Yamada]

Hi Arnd,

2016-03-29 17:11 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> On Tuesday 29 March 2016 15:59:00 kbuild test robot wrote:
>>
>> All warnings (new ones prefixed by >>):
>>
>> >> WARNING: vmlinux.o(.text+0x557b4): Section mismatch in reference from the function uniphier_cache_secondary_init() to the function .init.text:uniphier_cache_activate_all_ways()
>>    The function uniphier_cache_secondary_init() references
>>    the function __init uniphier_cache_activate_all_ways().
>>    This is often because uniphier_cache_secondary_init lacks a __init
>>    annotation or the annotation of uniphier_cache_activate_all_ways is wrong.
>>
>
>
> I guess the former: uniphier_cache_secondary_init should be __init, as it will
> only be run at boot time.
>
> Please resend both patches.
>

I think the alternative solution suggested by Russell is much better.

I hope Russell will apply his one, and then I retract this series.



-- 
Best Regards
Masahiro Yamada