On Sat, Jun 07, 2014 at 05:10:27PM +0100, Nicolas Pitre wrote:
> On Sat, 7 Jun 2014, Abhilash Kesavan wrote:
>
> > Hi Nicolas,
> >
> > The first man of the incoming cluster enables its snoops via the
> > power_up_setup function. During secondary boot-up, this does not occur
> > for the boot cluster. Hence, I enable the snoops for the boot cluster
> > as a one-time setup from the u-boot prompt. After secondary boot-up
> > there is no modification that I do.
>
> OK that's good.
>
> > Where should this be ideally done ?
>
> If I remember correctly, the CCI can be safely activated only when the
> cache is disabled. So that means the CCI should ideally be turned on
> for the boot cluster (and *only* for the boot CPU) by the bootloader.
CCI ports are enabled per-cluster, so the boot loader must turn on
CCI for all clusters before the respective CPUs have a chance to
turn on their caches. It is a secure operation, this can be overriden
and probably that's what has been done, wrongly.
True, TC2 on warm-boot reenables CCI, and that's because it runs the
kernel in secure world, and again that's _wrong_.
It must be done in firmware, and I am totally against any attempt at
papering over what looks like a messed up firmware implementation with
a bunch of hacks in the kernel, because that's what the patch below is
(soft restarting a CPU to enable CCI ? and adding generic code for that ?
what's next ?)
I understand there is an issue and lots at stake here, but I do not want the
patch below to be merged in the kernel, I am sorry, it is a tad too much.
Lorenzo
>
> Now... If you _really_ prefer to do it from the kernel to avoid
> difficulties with bootloader updates, then it should be possible to do
> it from the kernel by temporarily turning the cache off. This is not a
> small thing but the MCPM infrastructure can be leveraged. Here's what I
> tried on a TC2 which might just work for you as well:
>
> diff --git a/arch/arm/common/mcpm_entry.c b/arch/arm/common/mcpm_entry.c
> index 86fd60fefb..1cc49de308 100644
> --- a/arch/arm/common/mcpm_entry.c
> +++ b/arch/arm/common/mcpm_entry.c
> @@ -12,11 +12,13 @@
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/irqflags.h>
> +#include <linux/cpu_pm.h>
>
> #include <asm/mcpm.h>
> #include <asm/cacheflush.h>
> #include <asm/idmap.h>
> #include <asm/cputype.h>
> +#include <asm/suspend.h>
>
> extern unsigned long
> mcpm_entry_vectors[MAX_NR_CLUSTERS][MAX_CPUS_PER_CLUSTER];
>
> @@ -146,6 +148,44 @@ int mcpm_cpu_powered_up(void)
> return 0;
> }
>
> +static int go_down(unsigned long _arg)
> +{
> + void (*cache_disable)(void) = (void *)_arg;
> + unsigned int mpidr = read_cpuid_mpidr();
> + unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> + unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> + phys_reset_t phys_reset;
> +
> + mcpm_set_entry_vector(cpu, cluster, cpu_resume);
> + setup_mm_for_reboot();
> +
> + __mcpm_cpu_going_down(cpu, cluster);
> + BUG_ON(!__mcpm_outbound_enter_critical(cpu, cluster));
> + cache_disable();
> + __mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
> + __mcpm_cpu_down(cpu, cluster);
> +
> + phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
> + phys_reset(virt_to_phys(mcpm_entry_point));
> + BUG();
> +}
> +
> +int mcpm_loopback(void (*cache_disable)(void))
> +{
> + int ret;
> +
> + local_irq_disable();
> + local_fiq_disable();
> + ret = cpu_pm_enter();
> + if (!ret) {
> + ret = cpu_suspend((unsigned long)cache_disable, go_down);
> + cpu_pm_exit();
> + }
> + local_fiq_enable();
> + local_irq_enable();
> + return ret;
> +}
> +
> struct sync_struct mcpm_sync;
>
> /*
> diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
> index 29e7785a54..abecdd734f 100644
> --- a/arch/arm/mach-vexpress/tc2_pm.c
> +++ b/arch/arm/mach-vexpress/tc2_pm.c
> @@ -323,6 +323,26 @@ static void __naked tc2_pm_power_up_setup(unsigned int
> affinity_level)
> " b cci_enable_port_for_self ");
> }
>
> +int mcpm_loopback(void (*cache_disable)(void));
> +
> +static void tc2_cache_down(void)
> +{
> + pr_warn("TC2: disabling cache\n");
> + if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A15) {
> + /*
> + * On the Cortex-A15 we need to disable
> + * L2 prefetching before flushing the cache.
> + */
> + asm volatile(
> + "mcr p15, 1, %0, c15, c0, 3 \n\t"
> + "isb \n\t"
> + "dsb "
> + : : "r" (0x400) );
> + }
> + v7_exit_coherency_flush(all);
> + cci_disable_port_by_cpu(read_cpuid_mpidr());
> +}
> +
> static int __init tc2_pm_init(void)
> {
> int ret, irq;
> @@ -370,6 +390,7 @@ static int __init tc2_pm_init(void)
> ret = mcpm_platform_register(&tc2_pm_power_ops);
> if (!ret) {
> mcpm_sync_init(tc2_pm_power_up_setup);
> + BUG_ON(mcpm_loopback(tc2_cache_down) != 0);
> pr_info("TC2 power management initialized\n");
> }
> return ret;
>
> Of course it is not because I'm helping you sidestepping firmware
> problems that I'll stop shaming those who came up with that firmware
> design. ;-)
>
>
> Nicolas
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
