[PATCH 10/19] arm64: kvm: allows kvm cpu hotplug

On 01/27/2016 02:42 AM, James Morse wrote:

Hi!

On 15/01/16 19:18, Geoff Levand wrote:

quoted

From: AKASHI Takahiro <redacted>

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 <redacted>

quoted

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index e06fd29..e91f80e 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c

quoted

  #ifdef CONFIG_CPU_PM
  static int hyp_init_cpu_pm_notifier(struct notifier_block *self,
  				    unsigned long cmd,
  				    void *v)
  {
-	if (cmd == CPU_PM_EXIT &&
-	    __hyp_get_vectors() == hyp_default_vectors) {
-		cpu_init_hyp_mode(NULL);
+	switch (cmd) {
+	case CPU_PM_ENTER:
+		if (__this_cpu_read(kvm_arm_hardware_enabled))
+			cpu_reset_hyp_mode();
+
  		return NOTIFY_OK;
-	}
+	case CPU_PM_EXIT:
+		if (__this_cpu_read(kvm_arm_hardware_enabled))
+			cpu_init_hyp_mode();

I read this as:
		if (enabled)
			enable();

What am I missing? Is there a missing '!'?

[/me thinks some more]

I suspect this is trying to be clever: leaving the flag set over a
deep-sleep, to indicate that the hardware should be re-enabled when we
resume... if so, a comment to that effect would be good.

Yep, I meant so. Will add some comment.

quoted

-	return NOTIFY_DONE;
+		return NOTIFY_OK;
+
+	default:
+		return NOTIFY_DONE;
+	}
  }

  static struct notifier_block hyp_init_cpu_pm_nb = {

quoted

diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 3070096..bca79f9 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h

@@ -58,9 +58,18 @@

  #define HVC_CALL_FUNC 3

+/*
+ * HVC_RESET_CPU - Reset cpu in EL2 to initial state.
+ *
+ * @x0: entry address in trampoline code in va
+ * @x1: identical mapping page table in pa
+ */
+
  #define BOOT_CPU_MODE_EL1	(0xe11)
  #define BOOT_CPU_MODE_EL2	(0xe12)

+#define HVC_RESET_CPU 4
+

Patch 5 added a fancy new way to call arbitrary functions at el2, why
not use that? (it would save beating up el1_sync again).

Let me think. I need to detangle some header files.

I agree the trampoline stuff is complicated - I will try and cook-up a
version of this patch for hibernate that does this. (... and comment
what I think is happening above while I'm at it)

quoted

  #ifndef __ASSEMBLY__

  /*

diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S
index 1d7e502..d909ce2 100644
--- a/arch/arm64/kvm/hyp-init.S
+++ b/arch/arm64/kvm/hyp-init.S

@@ -140,6 +140,39 @@ merged:
  	eret
  ENDPROC(__kvm_hyp_init)

+	/*
+	 * x0: HYP boot pgd
+	 * x1: HYP phys_idmap_start
+	 */
+ENTRY(__kvm_hyp_reset)
+	/* We're in trampoline code in VA, switch back to boot page tables */
+	msr	ttbr0_el2, x0
+	isb
+
+	/* Invalidate the old TLBs */
+	tlbi	alle2
+	dsb	sy
+
+	/* Branch into PA space */
+	adr	x0, 1f
+	bfi	x1, x0, #0, #PAGE_SHIFT
+	br	x1
+
+	/* We're now in idmap, disable MMU */
+1:	mrs	x0, sctlr_el2
+	ldr	x1, =SCTLR_ELx_FLAGS
+	bic	x0, x0, x1		// Clear SCTL_M and etc
+	msr	sctlr_el2, x0
+	isb
+
+	/* Install stub vectors */
+	adrp	x0, __hyp_stub_vectors
+	add	x0, x0, #:lo12:__hyp_stub_vectors

adr_l ?

OK.

Thanks,
-Takahiro AKASHI

quoted

+	msr	vbar_el2, x0
+
+	eret
+ENDPROC(__kvm_hyp_reset)
+
  	.ltorg

  	.popsection

diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 15b1ef9..ed82dc2 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S

@@ -986,10 +991,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_CPU
+	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

adr_l ?

quoted

+	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. */
  	push	lr, xzr

  	/*

Thanks,

James