[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <561F7C03.9080206@linaro.org>
Date: Thu, 15 Oct 2015 19:12:19 +0900
From: AKASHI Takahiro <takahiro.akashi@...aro.org>
To: James Morse <james.morse@....com>
Cc: Catalin Marinas <Catalin.Marinas@....com>,
Will Deacon <Will.Deacon@....com>,
Marc Zyngier <Marc.Zyngier@....com>,
Mark Rutland <Mark.Rutland@....com>,
"christoffer.dall@...aro.org" <christoffer.dall@...aro.org>,
"geoff@...radead.org" <geoff@...radead.org>,
"broonie@...nel.org" <broonie@...nel.org>,
"david.griego@...aro.org" <david.griego@...aro.org>,
"freddy77@...il.com" <freddy77@...il.com>,
"kexec@...ts.infradead.org" <kexec@...ts.infradead.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linaro-kernel@...ts.linaro.org" <linaro-kernel@...ts.linaro.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 1/2] arm64: kvm: allows kvm cpu hotplug
James,
I reproduced the problem on Hikey board, but
On 10/13/2015 07:43 PM, James Morse wrote:
> Hi,
>
> On 13/10/15 06:38, AKASHI Takahiro wrote:
>> On 10/12/2015 10:28 PM, James Morse wrote:
>>> On 29/05/15 06:38, AKASHI Takahiro wrote:
>>>> The current kvm implementation on arm64 does cpu-specific initialization
>>>> at system boot, and has no way to gracefully shutdown a core in terms of
>>>> kvm. This prevents, especially, kexec from rebooting the system on a boot
>>>> core in EL2.
>>>>
>>>> This patch adds a cpu tear-down function and also puts an existing cpu-init
>>>> code into a separate function, kvm_arch_hardware_disable() and
>>>> kvm_arch_hardware_enable() respectively.
>>>> We don't need arm64-specific cpu hotplug hook any more.
>>>
>>> I think we do... on platforms where cpuidle uses psci to temporarily turn
>>> off cores that aren't in use, we lose the el2 state. This hotplug hook
>>> restores the state, even if there a no vms running.
>
> I've just noticed there are two cpu notifiers - we may be referring to
> different ones. (hyp_init_cpu_pm_nb and hyp_init_cpu_nb)
>
>
>> If I understand you correctly, with or without my patch, kvm doesn't work
>> under cpuidle anyway. Right?
>
> It works with, and without, v4.
> This patch v5 causes the problem.
>
>
>> If so, saving/restoring cpu states (or at least, kicking cpu hotplug hooks)
>> is cpuidle driver's responsibility, isn't it?
>
> Yes - but with v5, (at least one of) the hotplug hooks isn't having the
> same effect as before:
>
> Before v5, cpu_init_hyp_mode() is called via cpu_notify() each time
> cpu_suspend() suspends/wakes-up the core.
>
> Logically it should be the 'pm' notifier that does this work:
>> if (cmd == CPU_PM_EXIT &&
>> __hyp_get_vectors() == hyp_default_vectors) {
>> cpu_init_hyp_mode(NULL);
>> return NOTIFY_OK;
>>
>
> With v5, kvm_arch_hardware_enable() isn't called each time cpu_suspend()
> cycles the core.
Right. I misunderstood kvm_arm_get_running_vcpu().
> The problem appears to be this hunk, affecting the above code:
>> - if (cmd == CPU_PM_EXIT &&
>> - __hyp_get_vectors() == hyp_default_vectors) {
>> - cpu_init_hyp_mode(NULL);
>> + if (cmd == CPU_PM_EXIT && kvm_arm_get_running_vcpu()) {
>> + kvm_arch_hardware_enable();
>
> Changing this to just rename cpu_init_hyp_mode() to
> kvm_arch_hardware_enable() solves the problem.
The change that you suggested won't work well because kvm needs to maintain
cpu state with 'kvm_usage_count' using kvm_arch_hardware_enable/disable().
With this changed applied, you won't be able to do kexec.
I'm going to try more generic PM hook.
Thanks,
-Takahiro AKASHI
> Presumably kvm_arm_get_running_vcpu() evaluates to false before the first
> vm is started, meaning no vms can be started if pm events occur before
> starting the first vm.
>
> Sorry I blamed the wrong cpu notifier hook - I didn't realise there were two!
>
>
> Thanks,
>
> James
>
>
>>> This patch prevents me from running vms on such a platform, qemu gives:
>>>> kvm [1500]: Unsupported exception type: 6264688KVM internal error.
>>> Suberror: 0
>>>
>>> kvmtool goes with a more dramatic:
>>>> KVM exit reason: 17 ("KVM_EXIT_INTERNAL_ERROR")
>>>
>>> Disabling CONFIG_ARM_CPUIDLE solves this problem.
>>>
>>>
>>> (Sorry to revive an old thread - I've been using v4 of this patch for the
>>> hibernate/suspend-to-disk series).
>>>
>>>
>>>> Since this patch modifies common part of code between arm and arm64, one
>>>> stub definition, __cpu_reset_hyp_mode(), is added on arm side to avoid
>>>> compiling errors.
>>>>
>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@...aro.org>
>>>
>>>> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
>>>> index fd085ec..afe6263 100644
>>>> --- a/arch/arm64/kvm/hyp.S
>>>> +++ b/arch/arm64/kvm/hyp.S
>>>> @@ -1136,6 +1136,11 @@ ENTRY(kvm_call_hyp)
>>>> ret
>>>> ENDPROC(kvm_call_hyp)
>>>>
>>>> +ENTRY(kvm_call_reset)
>>>> + hvc #HVC_RESET
>>>> + ret
>>>> +ENDPROC(kvm_call_reset)
>>>> +
>>>> .macro invalid_vector label, target
>>>> .align 2
>>>> \label:
>>>> @@ -1179,10 +1184,27 @@ el1_sync: // Guest trapped
>>>> into EL2
>>>> cmp x18, #HVC_GET_VECTORS
>>>> b.ne 1f
>>>> mrs x0, vbar_el2
>>>> - b 2f
>>>> -
>>>> -1: /* Default to HVC_CALL_HYP. */
>>>> + b do_eret
>>>>
>>>> + /* jump into trampoline code */
>>>> +1: cmp x18, #HVC_RESET
>>>> + b.ne 2f
>>>> + /*
>>>> + * Entry point is:
>>>> + * TRAMPOLINE_VA
>>>> + * + (__kvm_hyp_reset - (__hyp_idmap_text_start & PAGE_MASK))
>>>> + */
>>>> + adrp x2, __kvm_hyp_reset
>>>> + add x2, x2, #:lo12:__kvm_hyp_reset
>>>> + adrp x3, __hyp_idmap_text_start
>>>> + add x3, x3, #:lo12:__hyp_idmap_text_start
>>>> + and x3, x3, PAGE_MASK
>>>> + sub x2, x2, x3
>>>> + ldr x3, =TRAMPOLINE_VA
>>>> + add x2, x2, x3
>>>> + br x2 // no return
>>>> +
>>>> +2: /* Default to HVC_CALL_HYP. */
>>>
>>> What was the reason not to use kvm_call_hyp(__kvm_hyp_reset, ...)?
>>> (You mentioned you wanted to at [0] - I can't find the details in the
>>> archive)
>>>
>>>
>>> Thanks,
>>>
>>> James
>>>
>>>
>>> [0] http://lists.infradead.org/pipermail/kexec/2015-April/335533.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists