From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Rutland Subject: Re: [PATCH 2/2] ARM: soft-reboot into same mode that we entered the kernel Date: Wed, 14 Dec 2016 12:17:47 +0000 Message-ID: <20161214121747.GH17982@leverpostej> References: <20161213113044.GC19985@leverpostej> <20161214115648.GG17982@leverpostej> <20161214120541.GD14217@n2100.armlinux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 889324022C for ; Wed, 14 Dec 2016 07:17:29 -0500 (EST) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dZOEHGG5kxt2 for ; Wed, 14 Dec 2016 07:17:28 -0500 (EST) Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 02FEE401E8 for ; Wed, 14 Dec 2016 07:17:27 -0500 (EST) Content-Disposition: inline In-Reply-To: <20161214120541.GD14217@n2100.armlinux.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Russell King - ARM Linux Cc: marc.zyngier@arm.com, linux-arm-kernel@lists.infradead.org, dave.martin@arm.com, kvmarm@lists.cs.columbia.edu List-Id: kvmarm@lists.cs.columbia.edu On Wed, Dec 14, 2016 at 12:05:41PM +0000, Russell King - ARM Linux wrote: > On Wed, Dec 14, 2016 at 11:56:49AM +0000, Mark Rutland wrote: > > On Wed, Dec 14, 2016 at 10:46:35AM +0000, Russell King wrote: > > > When we soft-reboot (eg, kexec) from one kernel into the next, we need > > > to ensure that we enter the new kernel in the same processor mode as > > > when we were entered, so that (eg) the new kernel can install its own > > > hypervisor - the old kernel's hypervisor will have been overwritten. > > > > > > In order to do this, we need to pass a flag to cpu_reset() so it knows > > > what to do, and we need to modify the kernel's own hypervisor stub to > > > allow it to handle a soft-reboot. > > > > > > As we are always guaranteed to install our own hypervisor if we're > > > entered in HYP32 mode, and KVM will have moved itself out of the way > > > on kexec/normal reboot, we can assume that our hypervisor is in place > > > when we want to kexec, so changing our hypervisor API should not be a > > > problem. > > > > Just to check, does that also hold true for kdump? > > > > I haven't gone digging yet, but it looks like KVM might still be > > installed, rather than the hyp stub, and we might need some logic to > > ensure that it's torn down... > > The same problem will be true of ARM64 - I don't see any solution to > that in the present state. Sure. We don't have kdump suppoort yet, and I intend for that to be addressed before kdump support is merged. > > > @@ -51,7 +52,9 @@ static void __soft_restart(void *addr) > > > > > > /* Switch to the identity mapping. */ > > > phys_reset = (phys_reset_t)virt_to_idmap(cpu_reset); > > > - phys_reset((unsigned long)addr); > > > + > > > + /* original stub should be restored by kvm */ > > > + phys_reset((unsigned long)addr, is_hyp_mode_available()); > > > > ... otherwise here we'd call into the KVM hyp code in a potentially > > confusing manner. > > > > Otherwise, this looks fine to me. > > The only thing that I can think which would resolve that would be to > lay down a standard API for the hyp code, so things like reboot into > hyp mode can work irrespective of the hyp stub in place. Sure; having the KVM hyp code also implement HVC_SOFT_RESTART seems sensible. This would also work for arm64. > The issue here is that a panic can happen at any time from any context > with any hyp stub in place, so there _needs_ to be a uniform way to do > this. It's very bad that we've got this far without this point having > been considered - all we can do right now is to try and fix the issues > as they come up. > > Right now, let's fix this so we get some kind of improvement, and later > try to sort out some kind of uniform interface for this task. Sure, that's a bigger task, and this is definitely a step in the right direction. We need to avoid the kdump regression somehow though; can we somehow detect if KVM is active and avoid issuing the HVC_SOFT_RESTART? Thanks, Mark.