On 12/11/2015 12:09 AM, AKASHI Takahiro wrote:
Yang,

On 12/11/2015 03:44 AM, Shi, Yang wrote:
On 12/3/2015 5:58 AM, Marc Zyngier wrote:
On 02/12/15 22:40, Ashwin Chaugule wrote:
Hello,

On 24 November 2015 at 17:25, Geoff Levand <[email protected]> wrote:
From: AKASHI Takahiro <[email protected]>

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.

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 <[email protected]>
---
  arch/arm/include/asm/kvm_host.h   | 10 ++++-
  arch/arm/include/asm/kvm_mmu.h    |  1 +
  arch/arm/kvm/arm.c                | 79
++++++++++++++++++---------------------
  arch/arm/kvm/mmu.c                |  5 +++
  arch/arm64/include/asm/kvm_host.h | 16 +++++++-
  arch/arm64/include/asm/kvm_mmu.h  |  1 +
  arch/arm64/include/asm/virt.h     |  9 +++++
  arch/arm64/kvm/hyp-init.S         | 33 ++++++++++++++++
  arch/arm64/kvm/hyp.S              | 32 ++++++++++++++--
  9 files changed, 138 insertions(+), 48 deletions(-)

[..]



  static struct notifier_block hyp_init_cpu_pm_nb = {
@@ -1108,11 +1119,6 @@ static int init_hyp_mode(void)
         }

         /*
-        * Execute the init code on each CPU.
-        */
-       on_each_cpu(cpu_init_hyp_mode, NULL, 1);
-
-       /*
          * Init HYP view of VGIC
          */
         err = kvm_vgic_hyp_init();

With this flow, the cpu_init_hyp_mode() is called only at VM guest
creation, but vgic_hyp_init() is called at bootup. On a system with
GICv3, it looks like we end up with bogus values from the ICH_VTR_EL2
(to get the number of LRs), because we're not reading it from EL2
anymore.

Indeed, this is completely broken (I just reproduced the issue on a
model). I wish this kind of details had been checked earlier, but thanks
for pointing it out.

Sorry for chiming in late. I did run into something similar when I
tested some earlier version patches with Akashi San.

The guest bootup just hangs with 4.1 kernel on my LS2085 board which
has GICv3. Not sure if this is the same issue. But,
my bisect did point to this patch.

Can you please try my fixup! patch to determine whether it is the same
issue or not?

I wish I could help this time, however, unfortunately, my LS2085 board had some hardware failure so I lost the access to it. Once I get a replacement or have it fixed, I will have a try.

Regards,
Yang


Thanks
-Takahiro AKASHI


I used to think it is my kernel's problem (4.1 sounds a little bit
old) since Akashi San can't reproduce this on his
Hikey board.

Thanks,
Yang


Whats the best way to fix this?
- Call kvm_arch_hardware_enable() before vgic_hyp_init() and disable
later?
- Fold the VGIC init stuff back into hardware_enable()?

None of that works - kvm_arch_hardware_enable() is called once per CPU,
while vgic_hyp_init() can only be called once. Also,
kvm_arch_hardware_enable() is called from interrupt context, and I
wouldn't feel comfortable starting probing DT and allocating stuff from
there.

- Read the VGIC number of LRs from the hyp stub?

That's may UNDEF if called in the wrong context. Also, this defeats the
point of stubs, which is just to install the vectors for the hypervisor.

- ..

Yeah, quite.

Geoff, Takahiro?

    M.




_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Reply via email to