Hi Lorenzo,
On 20/02/14 09:47, Lorenzo Pieralisi wrote:
> On Wed, Feb 19, 2014 at 09:34:21PM +0000, Rob Herring wrote:
>> On Wed, Feb 19, 2014 at 9:41 AM, Lorenzo Pieralisi
>
> [...]
>
>>> diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c
>>> index 6e51114..5034f7a 100644
>>> --- a/drivers/cpuidle/cpuidle-calxeda.c
>>> +++ b/drivers/cpuidle/cpuidle-calxeda.c
>>> @@ -41,9 +41,12 @@ static int calxeda_pwrdown_idle(struct cpuidle_device *dev,
>>> struct cpuidle_driver *drv,
>>> int index)
>>> {
>>> + int ret;
>>> +
>>> cpu_pm_enter();
>>> - cpu_suspend(0, calxeda_idle_finish);
>>> - cpu_pm_exit();
>>> + ret = cpu_suspend(0, calxeda_idle_finish);
>>> + if (!ret)
>>> + cpu_pm_exit();
>>
>> It seems a little strange that the enter does not have to be balanced
>> with an exit call. Couldn't the enter tear down things that need to be
>> re-enabled?
>
> That's why I mentioned it is not a proper fix, just wanted to check my
> assumptions. There are multiple solutions to this problem, but the only
> one which is proper IMHO is for KVM to either "reset registers" on
> cpu_pm_enter (CPU_PM_ENTER in hyp_init_cpu_pm_notifier), or to avoid
> executing the hyp init code if a proper registers settings is detected
> when the cpu_pm_exit is triggered (CPU_PM_EXIT).
>
> Is there a way for KVM to simply detect it has nothing to do (ie, detect
> hyp mode is already initialized and it has nothing to do) ?
>
> There is no way in the kernel to guarantee the processor has been reset,
> I could infer that from return paths (ie cpu_suspend returning 0) but
> that's still not correct.
We really need to distinguish two cases:
- CPU is coming back with its registers intact: nothing to do.
- CPU is coming back after having being reset and is coming back from
the secondary boot path: we need to reinstall the KVM vectors and stack.
>From EL1, there is absolutely no chance we can guess what is the state
of EL2. So I guess there is only two solutions:
- Get an indication that the CPU was reset or not: can we actually get
that information? I have the idea of a per-cpu variable indicating if
we're coming from reset of not, but that feels tricky. A hypercall
indicating the state of the HYP mode (free: re-init, engaged: do
nothing) feels slightly better.
- Always deactivate KVM on PM_ENTER: bloody expensive (requires
switching EL2 to some form of idmap, deactivate the MMU, reinstall the
original vectors), specially if we already know that this is useless.
So I'd favour the first solution. Thoughts?
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
