[U-Boot] [RESEND 1/3] imx: mx7: psci: add cpu hotplug support
Anson Huang
anson.huang at nxp.com
Wed Aug 8 06:02:31 UTC 2018
Hi, Stefan
Anson Huang
Best Regards!
> -----Original Message-----
> From: Stefan Agner [mailto:stefan at agner.ch]
> Sent: Wednesday, August 8, 2018 1:54 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
>
> 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%2Fpa
> >> tch
> 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?
I think most of people familiar with the Linux power management feature should
know the cpu kill flow/method, if there are other comments for this patch set, I will add
a detail note here in V3 patch set, if no comments, I think it should be also just fine, is that
OK for you?
Anson.
>
> --
> 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