[RFC PATCH] Add IPI entry for CPU UP

[RFC PATCH] Add IPI entry for CPU UP

[Zhaoyang Huang]

In some ARM SOCs, IPI interrupt is used for hotplug in one cpu, that is,
sending a IPI to the core in WFI and powerdown status. So Add a IPI
entry for handle this kind of cpu up interrupt

Signed-off-by: Zhaoyang Huang <zhaoyang.huang@spreadtrum.com>
---
 arch/arm64/kernel/smp.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b1adc51..20e63c9 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -70,6 +70,7 @@ enum ipi_msg_type {
 	IPI_CPU_STOP,
 	IPI_TIMER,
 	IPI_IRQ_WORK,
+	IPI_CPU_UP,
 };
 
 /*
@@ -627,6 +628,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
 	S(IPI_CPU_STOP, "CPU stop interrupts"),
 	S(IPI_TIMER, "Timer broadcast interrupts"),
 	S(IPI_IRQ_WORK, "IRQ work interrupts"),
+	S(IPI_CPU_UP, "Hotplug cpu up by ipi"),
 };
 
 static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
@@ -746,6 +748,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		irq_exit();
 		break;
 #endif
+       case IPI_CPU_UP:
+               break;
 
 	default:
 		pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
@@ -798,3 +802,9 @@ int setup_profiling_timer(unsigned int multiplier)
 {
 	return -EINVAL;
 }
+
+void smp_send_cpuup(int cpu)
+{
+       smp_cross_call(cpumask_of(cpu), IPI_CPU_UP);
+}
+
-- 
1.7.9.5

Re: [RFC PATCH] Add IPI entry for CPU UP

[Lorenzo Pieralisi]

On Mon, Jan 11, 2016 at 03:10:40PM +0800, Zhaoyang Huang wrote:
> In some ARM SOCs, IPI interrupt is used for hotplug in one cpu, that is,
> sending a IPI to the core in WFI and powerdown status. So Add a IPI
> entry for handle this kind of cpu up interrupt

On arm64 SOCs, with a mainline kernel, you can only hotplug CPUs out
and back in by using the PSCI firmware interface, which does not
require an IPI to boot a CPU, therefore this patch is useless.

Lorenzo

> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@spreadtrum.com>
> ---
>  arch/arm64/kernel/smp.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index b1adc51..20e63c9 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -70,6 +70,7 @@ enum ipi_msg_type {
>  	IPI_CPU_STOP,
>  	IPI_TIMER,
>  	IPI_IRQ_WORK,
> +	IPI_CPU_UP,
>  };
>  
>  /*
> @@ -627,6 +628,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
>  	S(IPI_CPU_STOP, "CPU stop interrupts"),
>  	S(IPI_TIMER, "Timer broadcast interrupts"),
>  	S(IPI_IRQ_WORK, "IRQ work interrupts"),
> +	S(IPI_CPU_UP, "Hotplug cpu up by ipi"),
>  };
>  
>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
> @@ -746,6 +748,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
>  		irq_exit();
>  		break;
>  #endif
> +       case IPI_CPU_UP:
> +               break;
>  
>  	default:
>  		pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
> @@ -798,3 +802,9 @@ int setup_profiling_timer(unsigned int multiplier)
>  {
>  	return -EINVAL;
>  }
> +
> +void smp_send_cpuup(int cpu)
> +{
> +       smp_cross_call(cpumask_of(cpu), IPI_CPU_UP);
> +}
> +
> -- 
> 1.7.9.5
> 

Re: [RFC PATCH] Add IPI entry for CPU UP

[Catalin Marinas]

On Mon, Jan 11, 2016 at 09:59:25AM +0000, Lorenzo Pieralisi wrote:
> On Mon, Jan 11, 2016 at 03:10:40PM +0800, Zhaoyang Huang wrote:
> > In some ARM SOCs, IPI interrupt is used for hotplug in one cpu, that is,
> > sending a IPI to the core in WFI and powerdown status. So Add a IPI
> > entry for handle this kind of cpu up interrupt
> 
> On arm64 SOCs, with a mainline kernel, you can only hotplug CPUs out
> and back in by using the PSCI firmware interface, which does not
> require an IPI to boot a CPU, therefore this patch is useless.

I fully agree.

BTW, such patches should cc linux-arm-kernel@lists.infradead.org as well
since they are ARM related.

-- 
Catalin

RE: [RFC PATCH] Add IPI entry for CPU UP

[Zhaoyang Huang (黄朝阳)]

________________________________________
From: Catalin Marinas <catalin.marinas@arm.com>
Sent: Monday, January 11, 2016 6:06 PM
To: Lorenzo Pieralisi
Cc: Zhaoyang Huang; Zhaoyang Huang (黄朝阳); will.deacon@arm.com; linux-kernel@vger.kernel.org; hanjun.guo@linaro.org; suzuki.poulose@arm.com; Mark Rutland
Subject: Re: [RFC PATCH] Add IPI entry for CPU UP

On Mon, Jan 11, 2016 at 09:59:25AM +0000, Lorenzo Pieralisi wrote:
> On Mon, Jan 11, 2016 at 03:10:40PM +0800, Zhaoyang Huang wrote:
> > In some ARM SOCs, IPI interrupt is used for hotplug in one cpu, that is,
> > sending a IPI to the core in WFI and powerdown status. So Add a IPI
> > entry for handle this kind of cpu up interrupt
>
> On arm64 SOCs, with a mainline kernel, you can only hotplug CPUs out
> and back in by using the PSCI firmware interface, which does not
> require an IPI to boot a CPU, therefore this patch is useless.

I fully agree.

BTW, such patches should cc linux-arm-kernel@lists.infradead.org as well
since they are ARM related.

Hi both,
In fact, this patch is related to the counterpart of the PSCI code in kernel world which you mentioned before. In SPRD's SOC, we have to implement a way of "wakeup" the core in powerdown state, which is to launch a IPI to the dest core. The reason why we can not accessing power related register to light on the core is the state machine of the PMU will not be safe for this scenario.
--
Catalin

Re: [RFC PATCH] Add IPI entry for CPU UP

[Mark Rutland]

On Mon, Jan 11, 2016 at 10:55:08AM +0000, Zhaoyang Huang (黄朝阳) wrote:
> 
> ________________________________________
> From: Catalin Marinas <catalin.marinas@arm.com>
> Sent: Monday, January 11, 2016 6:06 PM
> To: Lorenzo Pieralisi
> Cc: Zhaoyang Huang; Zhaoyang Huang (黄朝阳); will.deacon@arm.com; linux-kernel@vger.kernel.org; hanjun.guo@linaro.org; suzuki.poulose@arm.com; Mark Rutland
> Subject: Re: [RFC PATCH] Add IPI entry for CPU UP
> 
> On Mon, Jan 11, 2016 at 09:59:25AM +0000, Lorenzo Pieralisi wrote:
> > On Mon, Jan 11, 2016 at 03:10:40PM +0800, Zhaoyang Huang wrote:
> > > In some ARM SOCs, IPI interrupt is used for hotplug in one cpu, that is,
> > > sending a IPI to the core in WFI and powerdown status. So Add a IPI
> > > entry for handle this kind of cpu up interrupt
> >
> > On arm64 SOCs, with a mainline kernel, you can only hotplug CPUs out
> > and back in by using the PSCI firmware interface, which does not
> > require an IPI to boot a CPU, therefore this patch is useless.
> 
> I fully agree.
> 
> BTW, such patches should cc linux-arm-kernel@lists.infradead.org as well
> since they are ARM related.
> 
> Hi both,
> In fact, this patch is related to the counterpart of the PSCI code in
> kernel world which you mentioned before. In SPRD's SOC, we have to
> implement a way of "wakeup" the core in powerdown state, which is to
> launch a IPI to the dest core.

This is not required with PSCI, which abstracts the wakeup and power
management behind the CPU_ON call.

The kernel should only have to issue a CPU_ON call, and the firmware
should do the right thing behind the scenes (e.g. enabling power to the
core, sending an IPI if necessary).

If the kernel needs to do anything other than issue a CPU_ON call, this
is not PSCI.

> The reason why we can not accessing power related register to light on
> the core is the state machine of the PMU will not be safe for this
> scenario.

I'm not sure I understand.

Which software agent (kernel? firmware?) cannot access this PMU
register, and why?

What is the problem with the PMU state machine?

Thanks,
Mark.

Re: [RFC PATCH] Add IPI entry for CPU UP

[Zhaoyang Huang]

On 11 January 2016 at 19:03, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Jan 11, 2016 at 10:55:08AM +0000, Zhaoyang Huang (黄朝阳) wrote:
>>
>> ________________________________________
>> From: Catalin Marinas <catalin.marinas@arm.com>
>> Sent: Monday, January 11, 2016 6:06 PM
>> To: Lorenzo Pieralisi
>> Cc: Zhaoyang Huang; Zhaoyang Huang (黄朝阳); will.deacon@arm.com; linux-kernel@vger.kernel.org; hanjun.guo@linaro.org; suzuki.poulose@arm.com; Mark Rutland
>> Subject: Re: [RFC PATCH] Add IPI entry for CPU UP
>>
>> On Mon, Jan 11, 2016 at 09:59:25AM +0000, Lorenzo Pieralisi wrote:
>> > On Mon, Jan 11, 2016 at 03:10:40PM +0800, Zhaoyang Huang wrote:
>> > > In some ARM SOCs, IPI interrupt is used for hotplug in one cpu, that is,
>> > > sending a IPI to the core in WFI and powerdown status. So Add a IPI
>> > > entry for handle this kind of cpu up interrupt
>> >
>> > On arm64 SOCs, with a mainline kernel, you can only hotplug CPUs out
>> > and back in by using the PSCI firmware interface, which does not
>> > require an IPI to boot a CPU, therefore this patch is useless.
>>
>> I fully agree.
>>
>> BTW, such patches should cc linux-arm-kernel@lists.infradead.org as well
>> since they are ARM related.
>>
>> Hi both,
>> In fact, this patch is related to the counterpart of the PSCI code in
>> kernel world which you mentioned before. In SPRD's SOC, we have to
>> implement a way of "wakeup" the core in powerdown state, which is to
>> launch a IPI to the dest core.
>
> This is not required with PSCI, which abstracts the wakeup and power
> management behind the CPU_ON call.
>
> The kernel should only have to issue a CPU_ON call, and the firmware
> should do the right thing behind the scenes (e.g. enabling power to the
> core, sending an IPI if necessary).
>
> If the kernel needs to do anything other than issue a CPU_ON call, this
> is not PSCI.
>
>> The reason why we can not accessing power related register to light on
>> the core is the state machine of the PMU will not be safe for this
>> scenario.
>
> I'm not sure I understand.
>
> Which software agent (kernel? firmware?) cannot access this PMU
> register, and why?
>
> What is the problem with the PMU state machine?
>
With regarding to the cpu down, we use a so called "auto power down"
mode, which have the PMU power down the core after it detect WFI
status(in fact, it is the same method for cpu suspend for our SOC). By
using this kind of method of power down, we have to use the method
which I mentioned above for power on. In fact, we have ever used
another method of on/off, which have NOT the issue of launch IPI(we
call it as force shutdown). But it has some stability problem for cpu
on(PC will run out of range. ASIC engineers ask us to switch to auto
mode to solve it)

> Thanks,
> Mark.

Re: [RFC PATCH] Add IPI entry for CPU UP

[Mark Rutland]

On Mon, Jan 11, 2016 at 07:21:32PM +0800, Zhaoyang Huang wrote:
> On 11 January 2016 at 19:03, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Mon, Jan 11, 2016 at 10:55:08AM +0000, Zhaoyang Huang (黄朝阳) wrote:
> >> In fact, this patch is related to the counterpart of the PSCI code in
> >> kernel world which you mentioned before. In SPRD's SOC, we have to
> >> implement a way of "wakeup" the core in powerdown state, which is to
> >> launch a IPI to the dest core.
> >
> > This is not required with PSCI, which abstracts the wakeup and power
> > management behind the CPU_ON call.
> >
> > The kernel should only have to issue a CPU_ON call, and the firmware
> > should do the right thing behind the scenes (e.g. enabling power to the
> > core, sending an IPI if necessary).
> >
> > If the kernel needs to do anything other than issue a CPU_ON call, this
> > is not PSCI.
> >
> >> The reason why we can not accessing power related register to light on
> >> the core is the state machine of the PMU will not be safe for this
> >> scenario.
> >
> > I'm not sure I understand.
> >
> > Which software agent (kernel? firmware?) cannot access this PMU
> > register, and why?
> >
> > What is the problem with the PMU state machine?
> >
> With regarding to the cpu down, we use a so called "auto power down"
> mode, which have the PMU power down the core after it detect WFI
> status(in fact, it is the same method for cpu suspend for our SOC). By
> using this kind of method of power down, we have to use the method
> which I mentioned above for power on.

Even if you need an IPI to bring the CPU back online, I don't see why
this needs to be in the kernel. That can (and must) be done in the
firmware, hidden behind the PSCI interface.

The logical flow should be:

  CPU x: Kernel calls PSCI CPU_OFF
  CPU x: PSCI FW puts core in "auto power down" mode
  CPU x: PSCI FW issues a WFI
  CPU x: offline

  CPU y: Kernel calls PSCI CPU_ON for CPU x
  CPU y: PSCI FW sets up power controller
  CPU y: PSCI FW issues IPI to CPU x
  CPU y: PSCI FW waits for CPU x to come online

  CPU x: comes online
  CPU y: Returns to kernel

  CPU x: initialised by FW
  CPU x: enters kernel at provided entry point

Note that from the kernel's PoV, it only needs to call CPU_ON and
CPU_OFF.

> In fact, we have ever used another method of on/off, which have NOT
> the issue of launch IPI(we call it as force shutdown). But it has some
> stability problem for cpu on(PC will run out of range. ASIC engineers
> ask us to switch to auto mode to solve it)

I don't think this is relevant. See above.

I assume that the core is only placed in "auto power down" mode in the
firmware immediately before the WFI (i.e. a WFI in the kernel will not
trigger a power down spuriously)?

Thanks,
Mark.