[U-Boot] [RESEND 1/3] imx: mx7: psci: add cpu hotplug support

Wed Aug 8 00:38:20 UTC 2018

Hi, Stefan

Anson Huang
Best Regards!

> -----Original Message-----
> From: Stefan Agner [mailto:stefan at agner.ch]
> Sent: Tuesday, August 7, 2018 6:48 PM
> To: Anson Huang <anson.huang at nxp.com>
> Cc: sbabic at denx.de; Fabio Estevam <fabio.estevam at nxp.com>;
> albert.u.boot at aribaud.net; bryan.odonoghue at linaro.org; Stefan Agner
> <stefan.agner at toradex.com>; Peng Fan <peng.fan at nxp.com>;
> patrick.delaunay at st.com; sjg at chromium.org; Ye Li <ye.li at nxp.com>;
> u-boot at lists.denx.de; dl-linux-imx <linux-imx at nxp.com>
> Subject: Re: [U-Boot] [RESEND 1/3] imx: mx7: psci: add cpu hotplug support
> 
> Hi Anson,
> 
> Thanks for this patchset, looking forward to have full suspend/resume for i.MX
> 7 in mainline!
> 
> On 07.08.2018 08:46, Anson Huang wrote:
> > This patch adds cpu hotplug support, previous cpu_off implementation
> > is NOT safe, a CPU can NOT power down itself in runtime, it will cause
> > system bus hang due to pending transaction. So need to use other
> > online CPU to kill it when it is ready for killed.
> 
> How does this exactly show?

Such kind of issue has been showed a lot and verified on all i.MX6 SoCs and i.MX7D, the
best solution should be to let GPC module has ability to kill CPU by monitoring its
wfi signal when the cpu off request is triggered. A CPU runtime kills itself will cause
system bus hang when doing stress cpu hotplug test with multi tasks running in
background. Using handshake to let other online CPU to kill the CPU being offline
is a safe way, this is implemented in all our i.MX6 SoCs and i.MX7D in Linux kernel.

You can check the Linux kernel code for details:

arch/arm/mach-imx/hotplug.c  

> 
> It seemed to work somehow here?
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.ozlabs.org%2Fpatch%2F933948%2F&data=02%7C01%7CAnson.Hua
> ng%40nxp.com%7Cb302f3b035bd448819c708d5fc53502a%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C636692357116793775&sdata=90jsTS
> t9GdKA1f%2B%2BkUdGy3G8Cj2WnnGSarC34dZic0A%3D&reserved=0
> 

Simple test is passed, but stress test may fail.

> Is it guaranteed that psci_affinity_info gets called afterwards?
> 

Yes, the psci_affinity_info will be called 10 times with 10ms intervals until dying CPU being OFF, if
within 100ms it is still online, kernel will have warning message output, but 100ms is enough
for a CPU to be OFF, otherwise, there must be issue.

arch/arm/kernel/psci_smp.c
 98         for (i = 0; i < 10; i++) {
 99                 err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
100                 if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) {
101                         pr_info("CPU%d killed.\n", cpu);
102                         return 1;
103                 }
104
105                 msleep(10);
106                 pr_info("Retrying again to check for CPU kill\n");
107         }

> The solution seems somewhat hacky, but unfortunately I do not have a better
> idea...

Yes, we can treat it as a "software workaround", since our GPC does NOT have
hardware function to kill a CPU by monitoring its wfi signal, latest i.MX8 SoCs
have this feature added.

Anson. 

> 
> --
> Stefan
> 
> >
> > Here use SRC parameter register and a magic number of ~0 as handshake
> > for killing a offline CPU, when the online CPU checks the
> > psci_affinity_info, it will help kill the offline CPU according to the
> > magic number stored in SRC parameter register.
> >
> > Signed-off-by: Anson Huang <Anson.Huang at nxp.com>
> > ---
> >  arch/arm/mach-imx/mx7/psci-mx7.c | 23 +++++++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/mx7/psci-mx7.c
> > b/arch/arm/mach-imx/mx7/psci-mx7.c
> > index aae96c5..d6c4263 100644
> > --- a/arch/arm/mach-imx/mx7/psci-mx7.c
> > +++ b/arch/arm/mach-imx/mx7/psci-mx7.c
> > @@ -44,6 +44,14 @@
> >  #error "invalid value for CONFIG_ARMV7_PSCI_NR_CPUS"
> >  #endif
> >
> > +#define imx_cpu_gpr_entry_offset(cpu) \
> > +	(SRC_BASE_ADDR + SRC_GPR1_MX7D + cpu * 8) #define
> > +imx_cpu_gpr_para_offset(cpu) \
> > +	(imx_cpu_gpr_entry_offset(cpu) + 4)
> > +
> > +#define IMX_CPU_SYNC_OFF	~0
> > +#define IMX_CPU_SYNC_ON		0
> > +
> >  u8 psci_state[IMX7D_PSCI_NR_CPUS] __secure_data = {
> >  	 PSCI_AFFINITY_LEVEL_ON,
> >  	 PSCI_AFFINITY_LEVEL_OFF};
> > @@ -116,7 +124,7 @@ __secure s32 psci_cpu_on(u32 __always_unused
> > function_id, u32 mpidr, u32 ep,
> >
> >  	psci_save(cpu, ep, context_id);
> >
> > -	writel((u32)psci_cpu_entry, SRC_BASE_ADDR + cpu * 8 +
> SRC_GPR1_MX7D);
> > +	writel((u32)psci_cpu_entry, imx_cpu_gpr_entry_offset(cpu));
> >
> >  	psci_set_state(cpu, PSCI_AFFINITY_LEVEL_ON_PENDING);
> >
> > @@ -137,7 +145,11 @@ __secure s32 psci_cpu_off(void)
> >
> >  	imx_enable_cpu_ca7(cpu, false);
> >  	imx_gpcv2_set_core_power(cpu, false);
> > -	writel(0, SRC_BASE_ADDR + cpu * 8 + SRC_GPR1_MX7D + 4);
> > +	/*
> > +	 * We use the cpu jumping argument register to sync with
> > +	 * psci_affinity_info() which is running on cpu0 to kill the cpu.
> > +	 */
> > +	writel(IMX_CPU_SYNC_OFF, imx_cpu_gpr_para_offset(cpu));
> >
> >  	while (1)
> >  		wfi();
> > @@ -198,6 +210,13 @@ __secure s32 psci_affinity_info(u32
> > __always_unused function_id,
> >  	if (cpu >= IMX7D_PSCI_NR_CPUS)
> >  		return ARM_PSCI_RET_INVAL;
> >
> > +	/* CPU is waiting for killed */
> > +	if (readl(imx_cpu_gpr_para_offset(cpu)) == IMX_CPU_SYNC_OFF) {
> > +		imx_enable_cpu_ca7(cpu, false);
> > +		imx_gpcv2_set_core_power(cpu, false);
> > +		writel(IMX_CPU_SYNC_ON, imx_cpu_gpr_para_offset(cpu));
> > +	}
> > +
> >  	return psci_state[cpu];
> >  }