[U-Boot] [RESEND 1/3] imx: mx7: psci: add cpu hotplug support
Stefan Agner
stefan at agner.ch
Wed Aug 8 05:54:24 UTC 2018
On 08.08.2018 02:38, Anson Huang wrote:
> 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.
I see.
>
>
>> 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...
Thanks for the clarifications!
Can you maybe mention somewhere that the kernel will poll affinity info
after a CPU shutdown to make clear that this is guaranteed to be called?
--
Stefan
>
> 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];
>> > }
More information about the U-Boot
mailing list