[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 3/7] xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)

On 16/04/18 17:52, Mirela Simonovic wrote:
On Mon, Apr 16, 2018 at 4:26 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
Hi Mirela,

On 16/04/18 11:02, Mirela Simonovic wrote:

On Thu, Apr 12, 2018 at 3:31 PM, Julien Grall <julien.grall@xxxxxxx>
On 12/04/18 12:33, Mirela Simonovic wrote:
On Wed, Apr 11, 2018 at 4:46 PM, Julien Grall <julien.grall@xxxxxxx>
On 11/04/18 14:19, Mirela Simonovic wrote:

I disagree here, if you are unable to turn off a CPU via PSCI then something
is definitely wrong. This means that CPU will forever spin in Xen code with
no way to exit. This could bring interesting issue with anything potentially
modifying Xen code (i.e livepatching).

IHMO, the forever sleep in stop_cpu() is just a temporary solution to cater
shutdown of the platform. The state of secondary CPU does not much matter at
that time. In case of suspend/resume you want really want to be able to turn
off those CPUs correctly otherwise they are not going to come up again.

If we follow that logic the CPU will not be able to exit WFI state
either. So we should raise panic in that case as well and in cases
where the system is suspending + the CPU is stopped + cpu off doesn't
work so the CPU cannot be enabled again.

In case of suspend/resume you can check before hand whether we cpu_off is implemented for that platform. If not, you deny suspend.

This will avoid people using suspend/resume on those platforms.

However, raising panic makes no sense for shutdown scenario.

I agree and that's why I suggested to move the panic in one of my previous e-mail:

"Furthermore, now on platform without CPU off support (e.g non-PSCI platform and PSCI 0.1) you will log an error message that may worry people. In reality, PSCI cpu_off will unlikely fail, so you probably want to add a panic in call_psci_cpu_off instead."

How about we do it something like this:

void stop_cpu(void)
     if ( system_state == SYS_STATE_suspend )

     while ( 1 )

void call_psci_cpu_off(void)
     int errno;

     /* If successfull the cpu_off call doesn't return */
     errno = call_smc(PSCI_0_2_FN32_CPU_OFF, 0, 0, 0);
     if ( errno )
         panic("PSCI cpu off failed for CPU%d err=%d\n", get_processor_id(),

call_psci_cpu_off should not check for PSCI version because we need to
panic regardless.

Yes. There are no need for the check because it will never return on success.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.