Re: [PATCH v5 1/2] arm64: kvm: allows kvm cpu hotplug

[AKASHI Takahiro]

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.

If I understand you correctly, with or without my patch, kvm doesn't work
 under cpuidle anyway. Right?

If so, saving/restoring cpu states (or at least, kicking cpu hotplug hooks)
is cpuidle driver's responsibility, isn't it?

-Takahiro AKASHI

> 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@xxxxxxxxxx>
>
> > 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
>
>
> >   	push	lr, xzr
> >
> >   	/*
> > @@ -1196,7 +1218,9 @@ el1_sync:					// Guest trapped into EL2
> >   	blr	lr
> >
> >   	pop	lr, xzr
> > -2:	eret
> > +
> > +do_eret:
> > +	eret
> >
> >   el1_trap:
> >   	/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/