Re: [PATCH RFC 1/1] arm64: Use PSCI calls for CPU stop when hotplug is supported

[Pramod Kumar <pramod.kumar@broadcom.com>]

Thanks Sudeep for reviewing. Please see my comment inline below.

On Mon, Jan 21, 2019 at 4:52 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Mon, Jan 21, 2019 at 11:28:27AM +0530, Pramod Kumar wrote:
> > On Fri, Jan 18, 2019 at 5:02 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > On Fri, Jan 18, 2019 at 11:16:20AM +0530, Pramod Kumar wrote:
> > > > If CPU hotplug is supported, ipi_cpu_stop should use PSCI cpudie
> > > > call to stop the CPU. This call ensures L1/L2 cache flush,
> > > > CPUs cache-cohenrecy setting w.r.to interconnect.
> > >
> > > Firstly, this is not specific to PSCI and I don't see any PSCI calls as
> > > $subject claims.
> >
> > I had seen all the other cpu ops methods where only PSCI supports only
> > cpu_die features hence used PSCI reference in my subject line to be more
> > clear where this die gets converted in last.
> >
>
> OK, but since you are not directly dealing with PSCI APIs, it's misleading.
>

I can re-phrase my commit message which links explanation to PSCI.

> > > Next, you fail to explain why do you have to ensure
> > > caches are cleaned and why do you need that in ipi_cpu_stop ?
> > > What's the use case ?
> > >
> > Need comes from a specific use case where one Accelerator card(SoC) is
> > plugged in a sever over a PCIe interface.  This Card gets supply from a
> > battery, which could provide very less power for a very small time, in case
> > of any power loss. Once Card switches to battery, this has to reduce its
> > power consumption to its lowest point and back-up the DDR contents asap
> > before battery gets fully drained off.
> >
>
> OK, but I was expecting the complete call path for this use-case.

OK. Once power loss happens, Application processor gets interrupt and
it has to switch to MCU asap so that it can switch-off all irrelevant
power domain to reduce battery drain.
flow is like this-
Power loss interrupt --> interrupt handler -> send IPI to stop all
secondary CPUs-> switch primary CPU to ATF which ultimately transition
to MCU where all power off action is taken.

>
> > Since battery can provide limited power for a very short time hence need to
> > transition to lowest power. As per the transition process , CPUs power
> > domain has to be off but before that it needs to flush out its content to
> > system memory(L3) so that content could be backed-up by a MCU, a controller
> > consuming very less power. Since we can not afford plugging-out every
> > individual CPUs in sequence hence uses  ipi_cpu_stop for all other CPUs
>
> So, you are randomly using ipi_cpu_stop for something it's not supposed to
> be used. What do you exactly want to achieve in that context where you need
> to save power ?

Need to back-up DDR from MCU which will require L1/L2 content gets
flushed before CPUs getting shutdown.

>Why system off or reset not sufficient ?
System off or reset is not sufficient as we can not afford that much
delay in transition.

Flushing L1/L2 contents to System memory and Bringing out CPUs
Clusters out of coherency domain before shutting down Clusters puts
rest of system in sane state.  If cluster are not being taken out
properly from coherency domain before shutting down, it could lead
system un-reliable for rest parts.

> --
> Regards,
> Sudeep
>